diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b39302..635edde 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,28 @@ automatically from the `## [x.y.z]` heading matching `versionName` in `app/build ## [Unreleased] +## [1.2.0] - 2026-08-19 + +### Added +- **Coil now copes with a box that plays from more than one source.** A coming Phoniebox release + lets a box have a streaming service alongside its own music, and then the album list holds both. + An album's name and artist are no longer enough to say which one you meant — two sources can + offer the same record — so Coil now remembers where each album came from and tells the box, for + favourites and home screen shortcuts as well as the library. Without this, starting an album from + the streaming service would have emptied the queue and played nothing at all. Nothing changes on + a box with only its own music, and your existing favourites and covers carry over untouched +- **The library says which source each album came from**, on a box that has more than one. A small + icon marks whether an entry is an album, a playlist or a saved-songs collection, and the line + under the title names the source — so the same record held both on your box and in a streaming + account reads as two entries rather than one mysterious duplicate. A box with only its own music + shows none of this, because there it would be the same mark on every row +- **A setting for cover art that does not live on your box.** Phoniebox is gaining the ability to play + from a streaming service, and such a service keeps its cover art on its own servers rather than on + the box. Coil can now show those covers, but only if you ask it to: the switch sits under Appearance + and is **off by default**, because turning it on is the one thing in the app that makes your phone + talk to anything other than your box on the local network. Left off, those albums show a stand-in + cover and nothing leaves your network + ## [1.1.1] - 2026-08-12 ### Fixed diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 7e55ddd..c1208f0 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -20,8 +20,8 @@ android { applicationId = "app.coilforphoniebox" minSdk = 26 targetSdk = 36 - versionCode = 7 - versionName = "1.1.1" + versionCode = 8 + versionName = "1.2.0" } androidResources { diff --git a/app/src/main/kotlin/app/coilforphoniebox/ui/favorites/FavoritesScreen.kt b/app/src/main/kotlin/app/coilforphoniebox/ui/favorites/FavoritesScreen.kt index 3f9502c..951d1b9 100644 --- a/app/src/main/kotlin/app/coilforphoniebox/ui/favorites/FavoritesScreen.kt +++ b/app/src/main/kotlin/app/coilforphoniebox/ui/favorites/FavoritesScreen.kt @@ -95,6 +95,7 @@ fun FavoritesScreen( FavoriteEntry( favorite = favorite, box = state.activeBox, + allowExternalCovers = state.allowExternalCovers, compact = compact, coverPending = state.coverPending(favorite), link = viewModel.linkFor(favorite), @@ -118,6 +119,7 @@ fun FavoritesScreen( private fun FavoriteEntry( favorite: Favorite, box: PhonieBox?, + allowExternalCovers: Boolean, compact: Boolean, /** Whether this favourite is still waiting to hear whether the box has artwork for it. */ coverPending: Boolean, @@ -130,8 +132,8 @@ private fun FavoriteEntry( onLinkCopied: () -> Unit, ) { var menuOpen by remember { mutableStateOf(false) } - val coverUrl = remember(favorite.coverFile, box?.host) { - favorite.coverFile?.let { file -> box?.coverUrl(file) } + val coverUrl = remember(favorite.coverFile, box?.host, allowExternalCovers) { + favorite.coverFile?.let { ref -> box?.coverUrl(ref, allowExternalCovers) } } val menuButton: @Composable () -> Unit = { diff --git a/app/src/main/kotlin/app/coilforphoniebox/ui/favorites/FavoritesViewModel.kt b/app/src/main/kotlin/app/coilforphoniebox/ui/favorites/FavoritesViewModel.kt index 73a2d20..a606bf9 100644 --- a/app/src/main/kotlin/app/coilforphoniebox/ui/favorites/FavoritesViewModel.kt +++ b/app/src/main/kotlin/app/coilforphoniebox/ui/favorites/FavoritesViewModel.kt @@ -10,6 +10,7 @@ import app.coilforphoniebox.domain.model.Favorite import app.coilforphoniebox.domain.repository.BoxRepository import app.coilforphoniebox.domain.repository.FavoriteRepository import app.coilforphoniebox.domain.repository.PlayerRepository +import app.coilforphoniebox.domain.repository.SettingsRepository import app.coilforphoniebox.domain.usecase.PlayFavoriteUseCase import app.coilforphoniebox.domain.usecase.ResolveFavoriteCoverUseCase import app.coilforphoniebox.shortcuts.PlayDeepLink @@ -41,6 +42,7 @@ class FavoritesViewModel @Inject constructor( private val playFavorite: PlayFavoriteUseCase, private val resolveCover: ResolveFavoriteCoverUseCase, private val shortcuts: ShortcutPublisher, + settings: SettingsRepository, ) : ViewModel() { data class State( @@ -49,6 +51,8 @@ class FavoritesViewModel @Inject constructor( val connection: ConnectionState = ConnectionState.DISCONNECTED, /** Favourites whose cover lookup has finished — see [coverPending]. */ val coversSettled: Set = emptySet(), + /** Whether a cover held somewhere other than the box may be loaded (§16). */ + val allowExternalCovers: Boolean = false, ) { /** * Whether [favorite] is still waiting to hear whether it has artwork. @@ -96,12 +100,14 @@ class FavoritesViewModel @Inject constructor( }, player.connectionState, coversSettled, - ) { box, list, connection, settled -> + settings.settings.map { it.loadExternalCoverArt }.distinctUntilChanged(), + ) { box, list, connection, settled, allowExternalCovers -> State( favorites = list, activeBox = box, connection = connection, coversSettled = settled, + allowExternalCovers = allowExternalCovers, ) }.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), State()) diff --git a/app/src/main/kotlin/app/coilforphoniebox/ui/library/LibraryScreen.kt b/app/src/main/kotlin/app/coilforphoniebox/ui/library/LibraryScreen.kt index 4cb8dbd..5001978 100644 --- a/app/src/main/kotlin/app/coilforphoniebox/ui/library/LibraryScreen.kt +++ b/app/src/main/kotlin/app/coilforphoniebox/ui/library/LibraryScreen.kt @@ -31,10 +31,12 @@ import androidx.compose.material.icons.rounded.ArrowUpward import androidx.compose.material.icons.rounded.Folder import androidx.compose.material.icons.rounded.FolderOff import androidx.compose.material.icons.rounded.Info +import androidx.compose.material.icons.rounded.LibraryMusic import androidx.compose.material.icons.rounded.MoreVert import androidx.compose.material.icons.rounded.Close import androidx.compose.material.icons.rounded.MusicNote import androidx.compose.material.icons.rounded.PlayArrow +import androidx.compose.material.icons.rounded.QueueMusic import androidx.compose.material.icons.rounded.Search import androidx.compose.material.icons.rounded.SearchOff import androidx.compose.material.icons.rounded.Star @@ -58,6 +60,7 @@ import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.graphics.vector.ImageVector import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.input.ImeAction import androidx.compose.ui.text.style.TextOverflow @@ -66,9 +69,13 @@ import androidx.lifecycle.compose.collectAsStateWithLifecycle import app.coilforphoniebox.R import app.coilforphoniebox.domain.model.Box as PhonieBox import app.coilforphoniebox.domain.model.LibraryAlbum +import app.coilforphoniebox.domain.model.LibraryContentType import app.coilforphoniebox.domain.model.LibraryFolder +import app.coilforphoniebox.domain.model.LibraryProvider +import app.coilforphoniebox.domain.model.LibrarySource import app.coilforphoniebox.domain.model.LibraryTrack import app.coilforphoniebox.domain.model.PlayTarget +import app.coilforphoniebox.domain.model.key import app.coilforphoniebox.ui.components.ActionMenuItem import app.coilforphoniebox.ui.components.CoverArt import app.coilforphoniebox.ui.components.DetailRow @@ -175,12 +182,16 @@ private fun SearchResults(viewModel: LibraryViewModel) { val results by viewModel.searchResults.collectAsStateWithLifecycle() val favouriteKeys by viewModel.favoriteKeys.collectAsStateWithLifecycle() val activeBox by viewModel.activeBox.collectAsStateWithLifecycle() + val allowExternalCovers by viewModel.allowExternalCovers.collectAsStateWithLifecycle() + val showSources by viewModel.showSources.collectAsStateWithLifecycle() + val sources by viewModel.sources.collectAsStateWithLifecycle() var details by remember { mutableStateOf(null) } details?.let { target -> LibraryDetailsSheet( target = target, box = activeBox, + allowExternalCovers = allowExternalCovers, favouriteKeys = favouriteKeys, viewModel = viewModel, onDismiss = { details = null }, @@ -223,16 +234,19 @@ private fun SearchResults(viewModel: LibraryViewModel) { if (results.albums.isNotEmpty()) { item { ResultHeading(stringResource(R.string.library_tab_albums)) } - items(results.albums, key = { "album:${it.albumArtist}|${it.album}" }) { album -> + items(results.albums, key = { it.toPlayTarget().key }) { album -> AlbumRow( album = album, box = activeBox, - favourite = "album:${album.albumArtist}/${album.album}" in favouriteKeys, - onPlay = { viewModel.play(PlayTarget.Album(album.albumArtist, album.album)) }, + allowExternalCovers = allowExternalCovers, + showSource = showSources, + sources = sources, + favourite = album.toPlayTarget().key in favouriteKeys, + onPlay = { viewModel.play(album.toPlayTarget()) }, onToggleFavourite = { viewModel.toggleFavorite( label = album.album, - target = PlayTarget.Album(album.albumArtist, album.album), + target = album.toPlayTarget(), coverFile = album.coverFile, ) }, @@ -273,6 +287,9 @@ private fun FolderTab(viewModel: LibraryViewModel) { val state by viewModel.folderState.collectAsStateWithLifecycle() val favouriteKeys by viewModel.favoriteKeys.collectAsStateWithLifecycle() val activeBox by viewModel.activeBox.collectAsStateWithLifecycle() + val allowExternalCovers by viewModel.allowExternalCovers.collectAsStateWithLifecycle() + val showSources by viewModel.showSources.collectAsStateWithLifecycle() + val sources by viewModel.sources.collectAsStateWithLifecycle() val freshness = rememberFreshnessLabel(state.content.cachedAt) var details by remember { mutableStateOf(null) } @@ -287,6 +304,7 @@ private fun FolderTab(viewModel: LibraryViewModel) { LibraryDetailsSheet( target = target, box = activeBox, + allowExternalCovers = allowExternalCovers, favouriteKeys = favouriteKeys, viewModel = viewModel, onDismiss = { details = null }, @@ -555,14 +573,17 @@ private fun TrackRow( private fun AlbumRow( album: LibraryAlbum, box: PhonieBox?, + allowExternalCovers: Boolean, + showSource: Boolean, + sources: List, favourite: Boolean, onPlay: () -> Unit, onToggleFavourite: () -> Unit, onDetails: () -> Unit, ) { var menuOpen by remember { mutableStateOf(false) } - val coverUrl = remember(album.coverFile, box?.host) { - album.coverFile?.let { file -> box?.coverUrl(file) } + val coverUrl = remember(album.coverFile, box?.host, allowExternalCovers) { + album.coverFile?.let { ref -> box?.coverUrl(ref, allowExternalCovers) } } Surface( @@ -600,15 +621,29 @@ private fun AlbumRow( maxLines = 1, overflow = TextOverflow.Ellipsis, ) - Text( - text = album.albumArtist.ifBlank { - stringResource(R.string.library_unknown_artist) - }, - style = MaterialTheme.typography.bodySmall, - color = MaterialTheme.colorScheme.onSurfaceVariant, - maxLines = 1, - overflow = TextOverflow.Ellipsis, - ) + Row(verticalAlignment = Alignment.CenterVertically) { + // Inline rather than a corner badge: the 38 dp thumbnail beside it has no + // room for one, and the secondary line is free here in a way the grid's + // is not. + if (showSource) { + Icon( + imageVector = albumKindIcon(album.contentType), + contentDescription = albumKindLabel(album.contentType), + tint = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier + .size(13.dp) + .padding(end = 1.dp), + ) + Spacer(Modifier.size(4.dp)) + } + Text( + text = albumSubtitle(album, showSource, sources), + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + ) + } } if (favourite) { Icon( @@ -714,6 +749,9 @@ private fun AlbumTab(viewModel: LibraryViewModel) { val state by viewModel.albumState.collectAsStateWithLifecycle() val favouriteKeys by viewModel.favoriteKeys.collectAsStateWithLifecycle() val activeBox by viewModel.activeBox.collectAsStateWithLifecycle() + val allowExternalCovers by viewModel.allowExternalCovers.collectAsStateWithLifecycle() + val showSources by viewModel.showSources.collectAsStateWithLifecycle() + val sources by viewModel.sources.collectAsStateWithLifecycle() val freshness = rememberFreshnessLabel(state.cachedAt) var details by remember { mutableStateOf(null) } @@ -724,6 +762,7 @@ private fun AlbumTab(viewModel: LibraryViewModel) { LibraryDetailsSheet( target = target, box = activeBox, + allowExternalCovers = allowExternalCovers, favouriteKeys = favouriteKeys, viewModel = viewModel, onDismiss = { details = null }, @@ -747,18 +786,21 @@ private fun AlbumTab(viewModel: LibraryViewModel) { horizontalArrangement = Arrangement.spacedBy(12.dp), verticalArrangement = Arrangement.spacedBy(12.dp), ) { - items(state.albums, key = { "${it.albumArtist}|${it.album}" }) { album -> + items(state.albums, key = { it.toPlayTarget().key }) { album -> AlbumCell( album = album, box = activeBox, - favourite = "album:${album.albumArtist}/${album.album}" in favouriteKeys, + allowExternalCovers = allowExternalCovers, + showSource = showSources, + sources = sources, + favourite = album.toPlayTarget().key in favouriteKeys, coverPending = state.coverPending(album), onRequestCover = { viewModel.requestAlbumCover(album) }, - onPlay = { viewModel.play(PlayTarget.Album(album.albumArtist, album.album)) }, + onPlay = { viewModel.play(album.toPlayTarget()) }, onToggleFavourite = { viewModel.toggleFavorite( label = album.album, - target = PlayTarget.Album(album.albumArtist, album.album), + target = album.toPlayTarget(), coverFile = album.coverFile, ) }, @@ -784,6 +826,9 @@ private fun AlbumTab(viewModel: LibraryViewModel) { private fun AlbumCell( album: LibraryAlbum, box: PhonieBox?, + allowExternalCovers: Boolean, + showSource: Boolean, + sources: List, favourite: Boolean, coverPending: Boolean, onRequestCover: () -> Unit, @@ -799,8 +844,8 @@ private fun AlbumCell( if (album.coverFile == null) onRequestCover() } - val coverUrl = remember(album.coverFile, box?.host) { - album.coverFile?.let { file -> box?.coverUrl(file) } + val coverUrl = remember(album.coverFile, box?.host, allowExternalCovers) { + album.coverFile?.let { ref -> box?.coverUrl(ref, allowExternalCovers) } } Column( @@ -848,6 +893,10 @@ private fun AlbumCell( onDetails = onDetails, modifier = Modifier.align(Alignment.TopEnd), ) + // Bottom left: the two top corners are already the star and the menu. + if (showSource) { + AlbumKindBadge(album.contentType, Modifier.align(Alignment.BottomStart)) + } } Spacer(Modifier.height(6.dp)) Text( @@ -857,7 +906,7 @@ private fun AlbumCell( overflow = TextOverflow.Ellipsis, ) Text( - text = album.albumArtist.ifBlank { stringResource(R.string.library_unknown_artist) }, + text = albumSubtitle(album, showSource, sources), style = MaterialTheme.typography.bodySmall, color = MaterialTheme.colorScheme.onSurfaceVariant, maxLines = 1, @@ -867,6 +916,98 @@ private fun AlbumCell( } } +/** + * What kind of thing an entry is, as an icon. + * + * Deliberately **not** a heart or a star for a saved-songs collection: [Icons.Rounded.Star] + * already means "one of my favourites" everywhere else on this screen, and a second starry + * mark meaning something entirely different would read as the first one. + * + * An unrecognised kind — a backend Coil has never heard of — falls back to the album icon, + * which is what the albums list is mostly made of. + */ +private fun albumKindIcon(contentType: String): ImageVector = when (contentType) { + LibraryContentType.PLAYLIST -> Icons.Rounded.QueueMusic + LibraryContentType.COLLECTION -> Icons.Rounded.LibraryMusic + LibraryContentType.TRACK -> Icons.Rounded.MusicNote + else -> Icons.Rounded.Album +} + +/** Read out for [albumKindIcon], which is otherwise a mark with no name. */ +@Composable +private fun albumKindLabel(contentType: String): String = stringResource( + when (contentType) { + LibraryContentType.PLAYLIST -> R.string.library_kind_playlist + LibraryContentType.COLLECTION -> R.string.library_kind_collection + LibraryContentType.TRACK -> R.string.library_kind_track + else -> R.string.library_kind_album + }, +) + +/** + * What to call the backend an entry came from. + * + * Only the box's own library is named here. Anything else is called whatever the box calls + * it — which for a streaming service is a brand name that should not be translated — and an + * unknown backend that never reported a name falls back to its bare id, which is at least + * true. + */ +@Composable +private fun providerLabel(provider: String, sources: List): String = + if (provider == LibraryProvider.MPD) { + stringResource(R.string.library_source_local) + } else { + sources.firstOrNull { it.id == provider }?.label ?: provider + } + +/** + * The artist line, with the source appended where there is more than one to tell apart. + * + * The source is text rather than a second icon because a provider is a free-form name a box + * chooses for itself — a backend Coil has never heard of can still be named, but it cannot be + * given an icon that was not shipped. + */ +@Composable +private fun albumSubtitle( + album: LibraryAlbum, + showSource: Boolean, + sources: List, +): String { + val artist = album.albumArtist.ifBlank { stringResource(R.string.library_unknown_artist) } + if (!showSource) return artist + + val source = providerLabel(album.provider, sources) + // A streaming service names itself as the artist of its own saved-songs collection, which + // would otherwise read "Spotify · Spotify". Saying it once is saying it. + if (artist.equals(source, ignoreCase = true)) return artist + return "$artist · $source" +} + +/** + * The kind badge, over the artwork. + * + * On the tile there is nowhere else for it: the two lines underneath are the album and its + * artist, and the cover fills the rest. Given a scrim so it stays legible over artwork that + * happens to be pale. + */ +@Composable +private fun AlbumKindBadge(contentType: String, modifier: Modifier = Modifier) { + Surface( + shape = RoundedCornerShape(8.dp), + color = MaterialTheme.colorScheme.surface.copy(alpha = 0.85f), + modifier = modifier.padding(6.dp), + ) { + Icon( + imageVector = albumKindIcon(contentType), + contentDescription = albumKindLabel(contentType), + tint = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier + .padding(3.dp) + .size(16.dp), + ) + } +} + /** What a tab or the search results are currently showing details for. */ private sealed interface DetailsTarget { data class Folder(val folder: LibraryFolder) : DetailsTarget @@ -887,6 +1028,7 @@ private sealed interface DetailsTarget { private fun LibraryDetailsSheet( target: DetailsTarget, box: PhonieBox?, + allowExternalCovers: Boolean, favouriteKeys: Set, viewModel: LibraryViewModel, onDismiss: () -> Unit, @@ -927,15 +1069,16 @@ private fun LibraryDetailsSheet( is DetailsTarget.Album -> AlbumDetailsSheet( album = target.album, box = box, - favourite = "album:${target.album.albumArtist}/${target.album.album}" in favouriteKeys, + allowExternalCovers = allowExternalCovers, + favourite = target.album.toPlayTarget().key in favouriteKeys, onPlay = { onDismiss() - viewModel.play(PlayTarget.Album(target.album.albumArtist, target.album.album)) + viewModel.play(target.album.toPlayTarget()) }, onToggleFavourite = { viewModel.toggleFavorite( label = target.album.album, - target = PlayTarget.Album(target.album.albumArtist, target.album.album), + target = target.album.toPlayTarget(), coverFile = target.album.coverFile, ) }, @@ -1029,13 +1172,14 @@ private fun TrackDetailsSheet( private fun AlbumDetailsSheet( album: LibraryAlbum, box: PhonieBox?, + allowExternalCovers: Boolean, favourite: Boolean, onPlay: () -> Unit, onToggleFavourite: () -> Unit, onDismiss: () -> Unit, ) { - val coverUrl = remember(album.coverFile, box?.host) { - album.coverFile?.let { file -> box?.coverUrl(file) } + val coverUrl = remember(album.coverFile, box?.host, allowExternalCovers) { + album.coverFile?.let { ref -> box?.coverUrl(ref, allowExternalCovers) } } DetailsSheet( diff --git a/app/src/main/kotlin/app/coilforphoniebox/ui/library/LibraryViewModel.kt b/app/src/main/kotlin/app/coilforphoniebox/ui/library/LibraryViewModel.kt index 34ae346..863d4a9 100644 --- a/app/src/main/kotlin/app/coilforphoniebox/ui/library/LibraryViewModel.kt +++ b/app/src/main/kotlin/app/coilforphoniebox/ui/library/LibraryViewModel.kt @@ -8,11 +8,14 @@ import app.coilforphoniebox.domain.model.Favorite import app.coilforphoniebox.domain.model.FolderContent import app.coilforphoniebox.domain.model.LibraryAlbum import app.coilforphoniebox.domain.model.LibrarySearchResults +import app.coilforphoniebox.domain.model.LibrarySource import app.coilforphoniebox.domain.model.PlayTarget +import app.coilforphoniebox.domain.model.key import app.coilforphoniebox.domain.repository.BoxRepository import app.coilforphoniebox.domain.repository.FavoriteRepository import app.coilforphoniebox.domain.repository.LibraryRepository import app.coilforphoniebox.domain.repository.PlayerRepository +import app.coilforphoniebox.domain.repository.SettingsRepository import app.coilforphoniebox.ui.UiMessage import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -49,6 +52,7 @@ class LibraryViewModel @Inject constructor( private val boxes: BoxRepository, private val player: PlayerRepository, private val favorites: FavoriteRepository, + settings: SettingsRepository, ) : ViewModel() { data class FolderState( @@ -103,8 +107,43 @@ class LibraryViewModel @Inject constructor( val activeBox: StateFlow = boxes.activeBox.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), null) + /** + * Whether a cover the box points at somewhere other than itself may be loaded (§16). + * + * Travels beside [activeBox] because the two are always needed together: it takes both + * to turn a cover reference into a URL, and neither alone is enough. + */ + val allowExternalCovers: StateFlow = settings.settings + .map { it.loadExternalCoverArt } + .distinctUntilChanged() + .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), false) + private val activeBoxId = boxes.activeBox.map { it?.id }.distinctUntilChanged() + /** + * Where each entry came from, and whether that is worth saying at all. + * + * On a box with a single music source every entry is a local album, so the badge and the + * source name would be the same two marks on every row — noise that tells the user + * nothing. [showSources] is therefore false on such a box and the library looks exactly + * as it always has. + * + * It comes from the cached albums rather than from [sources], so it is right immediately + * on a cold start; [sources] only supplies the *names*, and only once a refresh has asked + * the box for them. + */ + val showSources: StateFlow = activeBoxId + .flatMapLatest { boxId -> + if (boxId == null) flowOf(false) else library.hasMultipleSources(boxId) + } + .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), false) + + val sources: StateFlow> = activeBoxId + .flatMapLatest { boxId -> + if (boxId == null) flowOf(emptyList()) else library.librarySources(boxId) + } + .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), emptyList()) + val folderState: StateFlow = combine( activeBoxId, currentPath, @@ -264,7 +303,7 @@ class LibraryViewModel @Inject constructor( fun requestAlbumCover(album: LibraryAlbum) { viewModelScope.launch { try { - library.ensureAlbumCover(album.boxId, album.albumArtist, album.album) + library.ensureAlbumCover(album) } finally { // Answered, so the cell may now stand something in if it came back empty. // The key carries the box id, which is why nothing needs clearing when the @@ -309,14 +348,7 @@ class LibraryViewModel @Inject constructor( folderRefreshing.value = false } - private fun favoriteKey(favorite: Favorite): String? = - favorite.toPlayTarget()?.let { keyFor(it) } - - private fun keyFor(target: PlayTarget): String = when (target) { - is PlayTarget.Folder -> "folder:${target.path}" - is PlayTarget.Album -> "album:${target.albumArtist}/${target.album}" - is PlayTarget.Track -> "track:${target.url}" - } + private fun favoriteKey(favorite: Favorite): String? = favorite.toPlayTarget()?.key private companion object { /** Short enough to feel live while typing, long enough to skip intermediate words. */ @@ -329,4 +361,7 @@ class LibraryViewModel @Inject constructor( * another's. Nothing needs clearing when the active box changes: those are simply other keys. */ private fun coverKeyOf(album: LibraryAlbum): String = - "${album.boxId}|${album.albumArtist}|${album.album}" + // The content URI belongs here too: two sources offering the same record are two tiles + // with two separate lookups, and a shared key would have the second stop waiting the + // moment the first was answered — showing a stand-in over artwork still on its way. + "${album.boxId}|${album.albumArtist}|${album.album}|${album.contentUri.orEmpty()}" diff --git a/app/src/main/kotlin/app/coilforphoniebox/ui/settings/SettingsScreen.kt b/app/src/main/kotlin/app/coilforphoniebox/ui/settings/SettingsScreen.kt index 11989a1..e8a8db2 100644 --- a/app/src/main/kotlin/app/coilforphoniebox/ui/settings/SettingsScreen.kt +++ b/app/src/main/kotlin/app/coilforphoniebox/ui/settings/SettingsScreen.kt @@ -115,6 +115,15 @@ fun SettingsScreen( onClick = { context.openAppLocaleSettings() }, ) + // Off by default and stated plainly, because turning it on is the only thing in the + // app that makes the phone talk to anything but the box (§16). + SwitchRow( + title = stringResource(R.string.settings_external_cover_art), + subtitle = stringResource(R.string.settings_external_cover_art_summary), + checked = state.settings.loadExternalCoverArt, + onCheckedChange = viewModel::setLoadExternalCoverArt, + ) + SectionDivider() SectionHeader(stringResource(R.string.settings_section_notification)) diff --git a/app/src/main/kotlin/app/coilforphoniebox/ui/settings/SettingsViewModel.kt b/app/src/main/kotlin/app/coilforphoniebox/ui/settings/SettingsViewModel.kt index 332a4bd..23c9a5b 100644 --- a/app/src/main/kotlin/app/coilforphoniebox/ui/settings/SettingsViewModel.kt +++ b/app/src/main/kotlin/app/coilforphoniebox/ui/settings/SettingsViewModel.kt @@ -73,6 +73,9 @@ class SettingsViewModel @Inject constructor( fun setDynamicColor(enabled: Boolean) = viewModelScope.launch { settings.setDynamicColor(enabled) } + fun setLoadExternalCoverArt(enabled: Boolean) = + viewModelScope.launch { settings.setLoadExternalCoverArt(enabled) } + fun setSessionMode(mode: SessionMode) { viewModelScope.launch { settings.setSessionMode(mode) diff --git a/app/src/main/res/values-de/strings.xml b/app/src/main/res/values-de/strings.xml index 63b10e3..54826e8 100644 --- a/app/src/main/res/values-de/strings.xml +++ b/app/src/main/res/values-de/strings.xml @@ -56,6 +56,11 @@ Noch keine Alben. Wenn du welche erwartest, lass die Box ihre Bibliothek neu einlesen. Unbekanntes Album Unbekannte Interpretin oder Interpret + Album + Playlist + Sammlung + Titel + Lokal Aktualisiert %1$s Noch nicht geladen gerade eben @@ -135,6 +140,8 @@ Ersetzt das Grün von Coil durch Farben aus deinem Hintergrundbild. Sprache Öffnet die Spracheinstellungen des Systems für Coil. + Cover von externen Diensten laden (z. B. Spotify) + Dein Telefon lädt diese Cover dann direkt beim Dienst. Sonst spricht Coil nur mit deiner Box, und diese Alben bekommen ein Ersatzcover. Boxen verwalten Adresse diff --git a/app/src/main/res/values-es/strings.xml b/app/src/main/res/values-es/strings.xml index 6062fc0..c9617a6 100644 --- a/app/src/main/res/values-es/strings.xml +++ b/app/src/main/res/values-es/strings.xml @@ -54,6 +54,11 @@ Todavía no hay álbumes. Si esperabas alguno, pide a la caja que vuelva a leer su biblioteca. Álbum desconocido Artista desconocido + Álbum + Lista de reproducción + Colección + Pista + Local Actualizado %1$s Aún no cargado ahora mismo @@ -136,6 +141,8 @@ Sustituye el verde de Coil por colores tomados de tu fondo de pantalla. Idioma Abre los ajustes de idioma del sistema para Coil. + Cargar carátulas de servicios externos (p. ej. Spotify) + Tu teléfono cargará esas carátulas del propio servicio. Si no, Coil solo habla con tu caja y esos álbumes muestran una carátula de sustitución. Gestionar cajas Dirección diff --git a/app/src/main/res/values-fr/strings.xml b/app/src/main/res/values-fr/strings.xml index 0d74089..c9553f4 100644 --- a/app/src/main/res/values-fr/strings.xml +++ b/app/src/main/res/values-fr/strings.xml @@ -58,6 +58,11 @@ Aucun album pour le moment. Si vous en attendez, demandez à la box de réanalyser sa bibliothèque. Album inconnu Artiste inconnu + Album + Playlist + Collection + Titre + Local Mis à jour %1$s Pas encore chargé à l\'instant @@ -140,6 +145,8 @@ Remplace le vert de Coil par des couleurs tirées de votre fond d\'écran. Langue Ouvre les réglages de langue du système pour Coil. + Charger les pochettes des services externes (p. ex. Spotify) + Votre téléphone charge alors ces pochettes auprès du service lui-même. Sinon, Coil ne parle qu\'à votre boîte, et ces albums affichent une pochette de remplacement. Gérer les box Adresse diff --git a/app/src/main/res/values-nl/strings.xml b/app/src/main/res/values-nl/strings.xml index 98c1416..0f68954 100644 --- a/app/src/main/res/values-nl/strings.xml +++ b/app/src/main/res/values-nl/strings.xml @@ -56,6 +56,11 @@ Nog geen albums. Laat de box haar bibliotheek opnieuw inlezen als je er wel verwacht. Onbekend album Onbekende artiest + Album + Afspeellijst + Verzameling + Nummer + Lokaal Bijgewerkt %1$s Nog niet geladen net nu @@ -135,6 +140,8 @@ Vervangt het groen van Coil door kleuren uit je achtergrond. Taal Opent de taalinstellingen van het systeem voor Coil. + Hoezen van externe diensten laden (bijv. Spotify) + Je telefoon laadt die hoezen dan rechtstreeks bij de dienst. Anders praat Coil alleen met je box en krijgen die albums een vervangende hoes. Boxen beheren Adres diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 0445700..e72cb2c 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -100,6 +100,20 @@ No albums yet. Rescan the library on the box if you expect some. Unknown album Unknown artist + + Album + Playlist + Collection + Track + + Local Updated %1$s Not loaded yet @@ -220,6 +234,8 @@ Replaces Coil\'s green with colours taken from your wallpaper. Language Opens the system language settings for Coil. + Load cover art for external services (e.g. Spotify) + Lets your phone load those covers from the service itself. Otherwise Coil only talks to your box, and those albums show a stand-in cover. diff --git a/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/FakeRepositories.kt b/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/FakeRepositories.kt index d534494..0b2ce90 100644 --- a/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/FakeRepositories.kt +++ b/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/FakeRepositories.kt @@ -12,6 +12,7 @@ import app.coilforphoniebox.domain.model.LibraryIndexResult import app.coilforphoniebox.domain.model.LibraryIndexState import app.coilforphoniebox.domain.model.JumpOutcome import app.coilforphoniebox.domain.model.LibrarySearchResults +import app.coilforphoniebox.domain.model.LibrarySource import app.coilforphoniebox.domain.model.PlayTarget import app.coilforphoniebox.domain.model.PlayerStatus import app.coilforphoniebox.domain.model.QueueEntry @@ -216,9 +217,25 @@ class FakeLibraryRepository( override fun albumsCachedAt(boxId: String): Flow = cachedAt + /** + * Swappable like the rest: a golden of a box with two music sources needs the names to + * label them with, and a single-source box reports none. + */ + val sources = MutableStateFlow(emptyList()) + + override fun librarySources(boxId: String): Flow> = sources + + /** + * Derived from the albums the same way the real one is, so a fixture cannot claim two + * sources while holding entries from one — the golden would then show a state the app + * cannot actually reach. + */ + override fun hasMultipleSources(boxId: String): Flow = + allAlbums.map { albums -> albums.map { it.provider }.distinct().size > 1 } + override suspend fun refreshAlbums(boxId: String): Result = Result.success(Unit) - override suspend fun ensureAlbumCover(boxId: String, albumArtist: String, album: String) = Unit + override suspend fun ensureAlbumCover(album: LibraryAlbum) = Unit /** * Null, deliberately: a golden has to be the same picture every run, and a cover that @@ -266,6 +283,10 @@ class FakeSettingsRepository(settings: AppSettings = AppSettings()) : SettingsRe state.value = state.value.copy(favoritesLayout = layout) } + override suspend fun setLoadExternalCoverArt(enabled: Boolean) { + state.value = state.value.copy(loadExternalCoverArt = enabled) + } + override suspend fun setOnboardingComplete(complete: Boolean) { state.value = state.value.copy(onboardingComplete = complete) } diff --git a/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/Fixtures.kt b/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/Fixtures.kt index 0a9dd84..0ea8ee8 100644 --- a/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/Fixtures.kt +++ b/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/Fixtures.kt @@ -5,8 +5,11 @@ import app.coilforphoniebox.domain.model.Favorite import app.coilforphoniebox.domain.model.FavoriteType import app.coilforphoniebox.domain.model.FolderContent import app.coilforphoniebox.domain.model.LibraryAlbum +import app.coilforphoniebox.domain.model.LibraryContentType import app.coilforphoniebox.domain.model.LibraryFolder +import app.coilforphoniebox.domain.model.LibraryProvider import app.coilforphoniebox.domain.model.LibrarySearchResults +import app.coilforphoniebox.domain.model.LibrarySource import app.coilforphoniebox.domain.model.LibraryTrack import app.coilforphoniebox.domain.model.PlaybackState import app.coilforphoniebox.domain.model.PlayerStatus @@ -170,12 +173,22 @@ object Fixtures { durationSeconds = seconds, ) - private fun album(artist: String, name: String, cover: String? = null) = LibraryAlbum( + private fun album( + artist: String, + name: String, + cover: String? = null, + provider: String = LibraryProvider.MPD, + contentUri: String? = null, + contentType: String = LibraryContentType.ALBUM, + ) = LibraryAlbum( boxId = livingRoom.id, albumArtist = artist, album = name, coverFile = cover, cachedAt = NOW, + provider = provider, + contentUri = contentUri, + contentType = contentType, ) /** @@ -193,6 +206,47 @@ object Fixtures { album("Sing-Along", "Songs for the Long Drive Home"), ) + /** + * A box with a streaming service alongside its own music — the only state in which the + * kind badge and the source name appear at all. + * + * Deliberately includes the awkward case the whole identity change exists for: "Ein Bär + * räumt auf" appears twice, once from each source. It has to stay two tiles, and the two + * have to be tellable apart — owning a record on disc *and* having it saved in an account + * is ordinary, and the two are started by different calls. + */ + val mixedSourceAlbums = listOf( + album("Bärenstark", "Ein Bär räumt auf", "cover-baer.jpg"), + album("Detective Stories", "The Missing Key", "cover-missing-key.jpg"), + album("Nursery Rhymes", "Wheels on the Bus", "cover-wheels.jpg"), + album( + artist = "Bärenstark", + name = "Ein Bär räumt auf", + provider = "spotify", + contentUri = "spotify:album:1", + ), + album( + artist = "Nico", + name = "Long Drive Home", + provider = "spotify", + contentUri = "spotify:playlist:2", + contentType = LibraryContentType.PLAYLIST, + ), + album( + artist = "Spotify", + name = "Liked Songs", + provider = "spotify", + contentUri = "spotify:collection:tracks", + contentType = LibraryContentType.COLLECTION, + ), + ) + + /** What such a box reports for itself; the labels are the box's own English. */ + val mixedSources = listOf( + LibrarySource(id = "mpd", label = "Local"), + LibrarySource(id = "spotify", label = "Spotify"), + ) + /** What "the" finds: one hit of each kind, which is the layout worth a golden. */ val searchResults = LibrarySearchResults( query = "the", diff --git a/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/LibraryScreenshotTest.kt b/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/LibraryScreenshotTest.kt index ba1a931..65c64e8 100644 --- a/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/LibraryScreenshotTest.kt +++ b/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/LibraryScreenshotTest.kt @@ -6,6 +6,7 @@ import app.coilforphoniebox.R import app.coilforphoniebox.domain.model.FolderContent import app.coilforphoniebox.domain.model.LibraryAlbum import app.coilforphoniebox.domain.model.LibrarySearchResults +import app.coilforphoniebox.domain.model.LibrarySource import app.coilforphoniebox.ui.library.LibraryScreen import app.coilforphoniebox.ui.library.LibraryViewModel import dagger.hilt.android.testing.HiltAndroidTest @@ -28,16 +29,18 @@ class LibraryScreenshotTest : ScreenshotTest() { albums: List = Fixtures.albums, albumsCachedAt: Long? = Fixtures.cachedThreeDaysAgo, searchResults: LibrarySearchResults = Fixtures.searchResults, + sources: List = emptyList(), ) = LibraryViewModel( library = FakeLibraryRepository( folders = folders, albums = albums, cachedAt = albumsCachedAt, results = searchResults, - ), + ).also { it.sources.value = sources }, boxes = FakeBoxRepository(), player = FakePlayerRepository(), favorites = FakeFavoriteRepository(Fixtures.favorites), + settings = FakeSettingsRepository(), ) private fun string(id: Int): String = RuntimeEnvironment.getApplication().getString(id) @@ -91,6 +94,21 @@ class LibraryScreenshotTest : ScreenshotTest() { captureRoot("library/albums_light") } + /** + * A box with a streaming service as well as its own music: every tile gains a kind badge + * and its source, and the same record from both sources stays two tiles. + * + * The counterpart is [albums_grid], which must keep showing none of that — on a box with + * one source the marks would be identical on every tile and say nothing. + */ + @Test + fun albums_grid_with_two_sources() { + val vm = viewModel(albums = Fixtures.mixedSourceAlbums, sources = Fixtures.mixedSources) + show { LibraryScreen(vm) } + compose.onNodeWithText(string(R.string.library_tab_albums)).performClick() + captureRoot("library/albums_two_sources_light") + } + /** * Results are debounced, so the clock has to move before there is anything to see — the * one place these tests have to acknowledge time at all. diff --git a/app/src/testDebug/screenshots/app/settings_lower_phone.png b/app/src/testDebug/screenshots/app/settings_lower_phone.png index 91e1ba5..d328d8b 100644 Binary files a/app/src/testDebug/screenshots/app/settings_lower_phone.png and b/app/src/testDebug/screenshots/app/settings_lower_phone.png differ diff --git a/app/src/testDebug/screenshots/app/settings_phone.png b/app/src/testDebug/screenshots/app/settings_phone.png index 6b06dcd..fa7b61f 100644 Binary files a/app/src/testDebug/screenshots/app/settings_phone.png and b/app/src/testDebug/screenshots/app/settings_phone.png differ diff --git a/app/src/testDebug/screenshots/app/settings_small.png b/app/src/testDebug/screenshots/app/settings_small.png index 9285b9a..1a98dbd 100644 Binary files a/app/src/testDebug/screenshots/app/settings_small.png and b/app/src/testDebug/screenshots/app/settings_small.png differ diff --git a/app/src/testDebug/screenshots/app/settings_tablet.png b/app/src/testDebug/screenshots/app/settings_tablet.png index 9f98139..f5f3aaa 100644 Binary files a/app/src/testDebug/screenshots/app/settings_tablet.png and b/app/src/testDebug/screenshots/app/settings_tablet.png differ diff --git a/app/src/testDebug/screenshots/library/albums_two_sources_light.png b/app/src/testDebug/screenshots/library/albums_two_sources_light.png new file mode 100644 index 0000000..7f85335 Binary files /dev/null and b/app/src/testDebug/screenshots/library/albums_two_sources_light.png differ diff --git a/core-data/build.gradle.kts b/core-data/build.gradle.kts index 2fde546..90bc130 100644 --- a/core-data/build.gradle.kts +++ b/core-data/build.gradle.kts @@ -22,6 +22,17 @@ android { kotlinOptions { jvmTarget = "17" } + + // The exported Room schemas, so `MigrationTestHelper` can build an old database and + // migrate it forwards. They live at the module root for KSP's benefit; this makes the + // same folder readable from a test. + sourceSets["test"].assets.srcDir("$projectDir/schemas") + + testOptions { + unitTests { + isIncludeAndroidResources = true + } + } } ksp { @@ -47,4 +58,8 @@ dependencies { testImplementation(libs.junit) testImplementation(libs.kotlinx.coroutines.test) + // Favourites are the one thing here a user cannot get back from the box, so every + // schema change is exercised against a real SQLite file rather than reasoned about. + testImplementation(libs.androidx.room.testing) + testImplementation(libs.robolectric) } diff --git a/core-data/schemas/app.coilforphoniebox.data.db.CoilDatabase/4.json b/core-data/schemas/app.coilforphoniebox.data.db.CoilDatabase/4.json new file mode 100644 index 0000000..67aeb01 --- /dev/null +++ b/core-data/schemas/app.coilforphoniebox.data.db.CoilDatabase/4.json @@ -0,0 +1,485 @@ +{ + "formatVersion": 1, + "database": { + "version": 4, + "identityHash": "9727dbec7be69aac904eff33e28605f5", + "entities": [ + { + "tableName": "boxes", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` TEXT NOT NULL, `displayName` TEXT NOT NULL, `host` TEXT NOT NULL, `rpcPort` INTEGER NOT NULL, `pubPort` INTEGER NOT NULL, `addedAt` INTEGER NOT NULL, `autoSessionEnabled` INTEGER NOT NULL, `networkSsid` TEXT, `lastSeenAt` INTEGER, `sortIndex` INTEGER NOT NULL, PRIMARY KEY(`id`))", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "displayName", + "columnName": "displayName", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "host", + "columnName": "host", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "rpcPort", + "columnName": "rpcPort", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "pubPort", + "columnName": "pubPort", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "addedAt", + "columnName": "addedAt", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "autoSessionEnabled", + "columnName": "autoSessionEnabled", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "networkSsid", + "columnName": "networkSsid", + "affinity": "TEXT", + "notNull": false + }, + { + "fieldPath": "lastSeenAt", + "columnName": "lastSeenAt", + "affinity": "INTEGER", + "notNull": false + }, + { + "fieldPath": "sortIndex", + "columnName": "sortIndex", + "affinity": "INTEGER", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "id" + ] + }, + "indices": [], + "foreignKeys": [] + }, + { + "tableName": "library_folders", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`boxId` TEXT NOT NULL, `path` TEXT NOT NULL, `parentPath` TEXT, `displayName` TEXT NOT NULL, `hasChildren` INTEGER NOT NULL, `searchText` TEXT NOT NULL DEFAULT '', `cachedAt` INTEGER NOT NULL, `contentCachedAt` INTEGER, PRIMARY KEY(`boxId`, `path`), FOREIGN KEY(`boxId`) REFERENCES `boxes`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )", + "fields": [ + { + "fieldPath": "boxId", + "columnName": "boxId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "path", + "columnName": "path", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "parentPath", + "columnName": "parentPath", + "affinity": "TEXT", + "notNull": false + }, + { + "fieldPath": "displayName", + "columnName": "displayName", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "hasChildren", + "columnName": "hasChildren", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "searchText", + "columnName": "searchText", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "''" + }, + { + "fieldPath": "cachedAt", + "columnName": "cachedAt", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "contentCachedAt", + "columnName": "contentCachedAt", + "affinity": "INTEGER", + "notNull": false + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "boxId", + "path" + ] + }, + "indices": [ + { + "name": "index_library_folders_boxId_parentPath", + "unique": false, + "columnNames": [ + "boxId", + "parentPath" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_library_folders_boxId_parentPath` ON `${TABLE_NAME}` (`boxId`, `parentPath`)" + } + ], + "foreignKeys": [ + { + "table": "boxes", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "boxId" + ], + "referencedColumns": [ + "id" + ] + } + ] + }, + { + "tableName": "library_tracks", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`boxId` TEXT NOT NULL, `url` TEXT NOT NULL, `parentPath` TEXT, `title` TEXT, `artist` TEXT, `album` TEXT, `trackNo` INTEGER, `durationSeconds` REAL, `searchText` TEXT NOT NULL DEFAULT '', PRIMARY KEY(`boxId`, `url`), FOREIGN KEY(`boxId`) REFERENCES `boxes`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )", + "fields": [ + { + "fieldPath": "boxId", + "columnName": "boxId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "url", + "columnName": "url", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "parentPath", + "columnName": "parentPath", + "affinity": "TEXT", + "notNull": false + }, + { + "fieldPath": "title", + "columnName": "title", + "affinity": "TEXT", + "notNull": false + }, + { + "fieldPath": "artist", + "columnName": "artist", + "affinity": "TEXT", + "notNull": false + }, + { + "fieldPath": "album", + "columnName": "album", + "affinity": "TEXT", + "notNull": false + }, + { + "fieldPath": "trackNo", + "columnName": "trackNo", + "affinity": "INTEGER", + "notNull": false + }, + { + "fieldPath": "durationSeconds", + "columnName": "durationSeconds", + "affinity": "REAL", + "notNull": false + }, + { + "fieldPath": "searchText", + "columnName": "searchText", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "''" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "boxId", + "url" + ] + }, + "indices": [ + { + "name": "index_library_tracks_boxId_parentPath", + "unique": false, + "columnNames": [ + "boxId", + "parentPath" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_library_tracks_boxId_parentPath` ON `${TABLE_NAME}` (`boxId`, `parentPath`)" + } + ], + "foreignKeys": [ + { + "table": "boxes", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "boxId" + ], + "referencedColumns": [ + "id" + ] + } + ] + }, + { + "tableName": "library_albums", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`boxId` TEXT NOT NULL, `albumArtist` TEXT NOT NULL, `album` TEXT NOT NULL, `coverFile` TEXT, `searchText` TEXT NOT NULL DEFAULT '', `cachedAt` INTEGER NOT NULL, `contentUri` TEXT NOT NULL DEFAULT '', `provider` TEXT NOT NULL DEFAULT 'mpd', `contentType` TEXT NOT NULL DEFAULT 'album', PRIMARY KEY(`boxId`, `albumArtist`, `album`, `contentUri`), FOREIGN KEY(`boxId`) REFERENCES `boxes`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )", + "fields": [ + { + "fieldPath": "boxId", + "columnName": "boxId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "albumArtist", + "columnName": "albumArtist", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "album", + "columnName": "album", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "coverFile", + "columnName": "coverFile", + "affinity": "TEXT", + "notNull": false + }, + { + "fieldPath": "searchText", + "columnName": "searchText", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "''" + }, + { + "fieldPath": "cachedAt", + "columnName": "cachedAt", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "contentUri", + "columnName": "contentUri", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "''" + }, + { + "fieldPath": "provider", + "columnName": "provider", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "'mpd'" + }, + { + "fieldPath": "contentType", + "columnName": "contentType", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "'album'" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "boxId", + "albumArtist", + "album", + "contentUri" + ] + }, + "indices": [ + { + "name": "index_library_albums_boxId", + "unique": false, + "columnNames": [ + "boxId" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_library_albums_boxId` ON `${TABLE_NAME}` (`boxId`)" + } + ], + "foreignKeys": [ + { + "table": "boxes", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "boxId" + ], + "referencedColumns": [ + "id" + ] + } + ] + }, + { + "tableName": "favorites", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `boxId` TEXT NOT NULL, `label` TEXT NOT NULL, `type` TEXT NOT NULL, `folder` TEXT, `albumArtist` TEXT, `album` TEXT, `trackUrl` TEXT, `provider` TEXT, `contentUri` TEXT, `coverFile` TEXT, `sortIndex` INTEGER NOT NULL, `launchCount` INTEGER NOT NULL, `shortcutPinned` INTEGER NOT NULL, FOREIGN KEY(`boxId`) REFERENCES `boxes`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "boxId", + "columnName": "boxId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "label", + "columnName": "label", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "type", + "columnName": "type", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "folder", + "columnName": "folder", + "affinity": "TEXT", + "notNull": false + }, + { + "fieldPath": "albumArtist", + "columnName": "albumArtist", + "affinity": "TEXT", + "notNull": false + }, + { + "fieldPath": "album", + "columnName": "album", + "affinity": "TEXT", + "notNull": false + }, + { + "fieldPath": "trackUrl", + "columnName": "trackUrl", + "affinity": "TEXT", + "notNull": false + }, + { + "fieldPath": "provider", + "columnName": "provider", + "affinity": "TEXT", + "notNull": false + }, + { + "fieldPath": "contentUri", + "columnName": "contentUri", + "affinity": "TEXT", + "notNull": false + }, + { + "fieldPath": "coverFile", + "columnName": "coverFile", + "affinity": "TEXT", + "notNull": false + }, + { + "fieldPath": "sortIndex", + "columnName": "sortIndex", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "launchCount", + "columnName": "launchCount", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "shortcutPinned", + "columnName": "shortcutPinned", + "affinity": "INTEGER", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": true, + "columnNames": [ + "id" + ] + }, + "indices": [ + { + "name": "index_favorites_boxId", + "unique": false, + "columnNames": [ + "boxId" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_favorites_boxId` ON `${TABLE_NAME}` (`boxId`)" + } + ], + "foreignKeys": [ + { + "table": "boxes", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "boxId" + ], + "referencedColumns": [ + "id" + ] + } + ] + } + ], + "views": [], + "setupQueries": [ + "CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)", + "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, '9727dbec7be69aac904eff33e28605f5')" + ] + } +} \ No newline at end of file diff --git a/core-data/src/main/kotlin/app/coilforphoniebox/data/db/CoilDatabase.kt b/core-data/src/main/kotlin/app/coilforphoniebox/data/db/CoilDatabase.kt index 8f98868..c0385a1 100644 --- a/core-data/src/main/kotlin/app/coilforphoniebox/data/db/CoilDatabase.kt +++ b/core-data/src/main/kotlin/app/coilforphoniebox/data/db/CoilDatabase.kt @@ -11,7 +11,7 @@ import androidx.room.RoomDatabase LibraryAlbumEntity::class, FavoriteEntity::class, ], - version = 3, + version = 4, exportSchema = true, ) abstract class CoilDatabase : RoomDatabase() { diff --git a/core-data/src/main/kotlin/app/coilforphoniebox/data/db/Entities.kt b/core-data/src/main/kotlin/app/coilforphoniebox/data/db/Entities.kt index 69fb2df..bf608ac 100644 --- a/core-data/src/main/kotlin/app/coilforphoniebox/data/db/Entities.kt +++ b/core-data/src/main/kotlin/app/coilforphoniebox/data/db/Entities.kt @@ -89,9 +89,18 @@ data class LibraryTrackEntity( @ColumnInfo(defaultValue = "") val searchText: String, ) +/** + * One row of the albums list. + * + * [contentUri] is part of the key because artist-and-album stopped being unique the moment a + * box could have two backends: the same record can be both on the box's disk and in a + * streaming account, and the two are played by different calls. It is `""` rather than null + * for everything MPD owns — a primary key column cannot be null, and the domain model maps + * the empty string back to null at the boundary. + */ @Entity( tableName = "library_albums", - primaryKeys = ["boxId", "albumArtist", "album"], + primaryKeys = ["boxId", "albumArtist", "album", "contentUri"], foreignKeys = [ ForeignKey( entity = BoxEntity::class, @@ -110,6 +119,10 @@ data class LibraryAlbumEntity( /** Folded album and album artist; see [SearchText]. */ @ColumnInfo(defaultValue = "") val searchText: String, val cachedAt: Long, + /** `""` for MPD, which has no handle of its own — see the note on the entity. */ + @ColumnInfo(defaultValue = "") val contentUri: String = "", + @ColumnInfo(defaultValue = "mpd") val provider: String = "mpd", + @ColumnInfo(defaultValue = "album") val contentType: String = "album", ) @Entity( @@ -135,6 +148,15 @@ data class FavoriteEntity( val album: String?, /** MPD URL of a single file, set only for a `TRACK` row (schema version 2). */ val trackUrl: String?, + /** + * Which backend owns an `ALBUM` row, and that backend's handle for it (schema version 4). + * + * Nullable rather than defaulted, because null is the true answer for every favourite + * saved before a box could have more than one backend, and for every folder and track + * row. A null reads as MPD, which is what those rows are. + */ + val provider: String?, + val contentUri: String?, val coverFile: String?, val sortIndex: Int, val launchCount: Int, diff --git a/core-data/src/main/kotlin/app/coilforphoniebox/data/db/FavoriteDao.kt b/core-data/src/main/kotlin/app/coilforphoniebox/data/db/FavoriteDao.kt index d065c87..27375c9 100644 --- a/core-data/src/main/kotlin/app/coilforphoniebox/data/db/FavoriteDao.kt +++ b/core-data/src/main/kotlin/app/coilforphoniebox/data/db/FavoriteDao.kt @@ -25,11 +25,23 @@ interface FavoriteDao { ) suspend fun findFolder(boxId: String, folder: String): FavoriteEntity? + /** + * `IS` rather than `=` for the content URI: it is null on every album favourite saved + * before the column existed, and `= NULL` is never true in SQL — so `=` would fail to + * match precisely the rows that predate it, and starring such an album again would add a + * duplicate instead of unstarring it. + */ @Query( "SELECT * FROM favorites WHERE boxId = :boxId AND type = 'ALBUM' " + - "AND albumArtist = :albumArtist AND album = :album LIMIT 1", + "AND albumArtist = :albumArtist AND album = :album " + + "AND contentUri IS :contentUri LIMIT 1", ) - suspend fun findAlbum(boxId: String, albumArtist: String, album: String): FavoriteEntity? + suspend fun findAlbum( + boxId: String, + albumArtist: String, + album: String, + contentUri: String?, + ): FavoriteEntity? @Query( "SELECT * FROM favorites WHERE boxId = :boxId AND type = 'TRACK' AND trackUrl = :trackUrl LIMIT 1", diff --git a/core-data/src/main/kotlin/app/coilforphoniebox/data/db/LibraryDao.kt b/core-data/src/main/kotlin/app/coilforphoniebox/data/db/LibraryDao.kt index 62451ec..f0971cb 100644 --- a/core-data/src/main/kotlin/app/coilforphoniebox/data/db/LibraryDao.kt +++ b/core-data/src/main/kotlin/app/coilforphoniebox/data/db/LibraryDao.kt @@ -162,16 +162,20 @@ interface LibraryDao { @Transaction suspend fun replaceAlbums(boxId: String, albums: List) { // Covers are fetched one at a time as the grid scrolls, so keep the ones already - // resolved rather than making the whole grid ask again after every refresh. - val existingCovers = coversFor(boxId).associate { (it.albumArtist to it.album) to it.coverFile } + // resolved rather than making the whole grid ask again after every refresh. Keyed on + // the whole identity: with two backends the artist-and-album pair can name two rows, + // and a narrower key would hand one of them the other's artwork. + val existingCovers = coversFor(boxId).associate { it.identity() to it.coverFile } deleteAlbums(boxId) upsertAlbums( albums.map { album -> - album.copy(coverFile = album.coverFile ?: existingCovers[album.albumArtist to album.album]) + album.copy(coverFile = album.coverFile ?: existingCovers[album.identity()]) }, ) } + private fun LibraryAlbumEntity.identity() = Triple(albumArtist, album, contentUri) + @Query("SELECT * FROM library_albums WHERE boxId = :boxId AND coverFile IS NOT NULL") suspend fun coversFor(boxId: String): List @@ -183,10 +187,29 @@ interface LibraryDao { @Query( "UPDATE library_albums SET coverFile = :coverFile " + - "WHERE boxId = :boxId AND albumArtist = :albumArtist AND album = :album", + "WHERE boxId = :boxId AND albumArtist = :albumArtist AND album = :album " + + "AND contentUri = :contentUri", + ) + suspend fun setAlbumCover( + boxId: String, + albumArtist: String, + album: String, + contentUri: String, + coverFile: String, ) - suspend fun setAlbumCover(boxId: String, albumArtist: String, album: String, coverFile: String) - @Query("SELECT * FROM library_albums WHERE boxId = :boxId AND albumArtist = :albumArtist AND album = :album") - suspend fun findAlbum(boxId: String, albumArtist: String, album: String): LibraryAlbumEntity? + @Query( + "SELECT * FROM library_albums WHERE boxId = :boxId AND albumArtist = :albumArtist " + + "AND album = :album AND contentUri = :contentUri", + ) + suspend fun findAlbum( + boxId: String, + albumArtist: String, + album: String, + contentUri: String, + ): LibraryAlbumEntity? + + /** How many distinct backends the cached albums came from; 1 on any single-backend box. */ + @Query("SELECT COUNT(DISTINCT provider) FROM library_albums WHERE boxId = :boxId") + fun observeProviderCount(boxId: String): Flow } diff --git a/core-data/src/main/kotlin/app/coilforphoniebox/data/db/Mappers.kt b/core-data/src/main/kotlin/app/coilforphoniebox/data/db/Mappers.kt index 222c7ea..78078b8 100644 --- a/core-data/src/main/kotlin/app/coilforphoniebox/data/db/Mappers.kt +++ b/core-data/src/main/kotlin/app/coilforphoniebox/data/db/Mappers.kt @@ -5,6 +5,7 @@ import app.coilforphoniebox.domain.model.Favorite import app.coilforphoniebox.domain.model.FavoriteType import app.coilforphoniebox.domain.model.LibraryAlbum import app.coilforphoniebox.domain.model.LibraryFolder +import app.coilforphoniebox.domain.model.LibraryProvider import app.coilforphoniebox.domain.model.LibraryTrack internal fun BoxEntity.toDomain() = Box( @@ -85,6 +86,10 @@ internal fun LibraryAlbumEntity.toDomain() = LibraryAlbum( album = album, coverFile = coverFile, cachedAt = cachedAt, + provider = provider, + // `""` is how "no handle of its own" is stored, since a key column cannot be null. + contentUri = contentUri.takeIf { it.isNotEmpty() }, + contentType = contentType, ) internal fun LibraryAlbum.toEntity() = LibraryAlbumEntity( @@ -94,6 +99,9 @@ internal fun LibraryAlbum.toEntity() = LibraryAlbumEntity( coverFile = coverFile, searchText = SearchText.haystack(album, albumArtist), cachedAt = cachedAt, + contentUri = contentUri.orEmpty(), + provider = provider, + contentType = contentType, ) internal fun FavoriteEntity.toDomain() = Favorite( @@ -107,6 +115,9 @@ internal fun FavoriteEntity.toDomain() = Favorite( albumArtist = albumArtist, album = album, trackUrl = trackUrl, + // A row saved before backends existed has no provider, and MPD is what it was. + provider = provider ?: LibraryProvider.MPD, + contentUri = contentUri, coverFile = coverFile, sortIndex = sortIndex, launchCount = launchCount, @@ -122,6 +133,8 @@ internal fun Favorite.toEntity() = FavoriteEntity( albumArtist = albumArtist, album = album, trackUrl = trackUrl, + provider = provider, + contentUri = contentUri, coverFile = coverFile, sortIndex = sortIndex, launchCount = launchCount, diff --git a/core-data/src/main/kotlin/app/coilforphoniebox/data/db/Migrations.kt b/core-data/src/main/kotlin/app/coilforphoniebox/data/db/Migrations.kt index d8c0418..e5e2f67 100644 --- a/core-data/src/main/kotlin/app/coilforphoniebox/data/db/Migrations.kt +++ b/core-data/src/main/kotlin/app/coilforphoniebox/data/db/Migrations.kt @@ -38,4 +38,62 @@ internal val MIGRATION_2_3 = object : Migration(2, 3) { } } -internal val ALL_MIGRATIONS = arrayOf(MIGRATION_1_2, MIGRATION_2_3) +/** + * Album identity gains the backend that owns it. + * + * A Phoniebox can now register more than one player backend, and the albums call returns all + * of their catalogues in one list. Artist-and-album is no longer unique across it, so the + * cache is re-keyed on the content URI as well. + * + * The two tables are treated differently on purpose: + * + * - `library_albums` needs a new primary key, which SQLite cannot alter in place, so the + * table is rebuilt. The rows are **copied rather than dropped**, unlike [MIGRATION_2_3]: + * every existing row necessarily came from a box with one backend, so `'mpd'` and `''` are + * not fallback defaults but the correct values, and copying keeps the resolved covers that + * `LibraryDao.replaceAlbums` goes to some trouble to preserve. + * - `favorites` only gains two nullable columns, the way `trackUrl` arrived in + * [MIGRATION_1_2]. Favourites are the one thing here a user cannot get back from the box, + * so nothing about that table is rebuilt, and null is the honest value for a row saved + * before any of this existed. + */ +internal val MIGRATION_3_4 = object : Migration(3, 4) { + override fun migrate(db: SupportSQLiteDatabase) { + db.execSQL( + """ + CREATE TABLE IF NOT EXISTS `library_albums_new` ( + `boxId` TEXT NOT NULL, + `albumArtist` TEXT NOT NULL, + `album` TEXT NOT NULL, + `coverFile` TEXT, + `searchText` TEXT NOT NULL DEFAULT '', + `cachedAt` INTEGER NOT NULL, + `contentUri` TEXT NOT NULL DEFAULT '', + `provider` TEXT NOT NULL DEFAULT 'mpd', + `contentType` TEXT NOT NULL DEFAULT 'album', + PRIMARY KEY(`boxId`, `albumArtist`, `album`, `contentUri`), + FOREIGN KEY(`boxId`) REFERENCES `boxes`(`id`) + ON UPDATE NO ACTION ON DELETE CASCADE + ) + """.trimIndent(), + ) + db.execSQL( + """ + INSERT OR IGNORE INTO `library_albums_new` + (`boxId`, `albumArtist`, `album`, `coverFile`, `searchText`, `cachedAt`, + `contentUri`, `provider`, `contentType`) + SELECT `boxId`, `albumArtist`, `album`, `coverFile`, `searchText`, `cachedAt`, + '', 'mpd', 'album' + FROM `library_albums` + """.trimIndent(), + ) + db.execSQL("DROP TABLE `library_albums`") + db.execSQL("ALTER TABLE `library_albums_new` RENAME TO `library_albums`") + db.execSQL("CREATE INDEX IF NOT EXISTS `index_library_albums_boxId` ON `library_albums` (`boxId`)") + + db.execSQL("ALTER TABLE `favorites` ADD COLUMN `provider` TEXT") + db.execSQL("ALTER TABLE `favorites` ADD COLUMN `contentUri` TEXT") + } +} + +internal val ALL_MIGRATIONS = arrayOf(MIGRATION_1_2, MIGRATION_2_3, MIGRATION_3_4) diff --git a/core-data/src/main/kotlin/app/coilforphoniebox/data/repository/BackupRepositoryImpl.kt b/core-data/src/main/kotlin/app/coilforphoniebox/data/repository/BackupRepositoryImpl.kt index f5837ea..50e72d3 100644 --- a/core-data/src/main/kotlin/app/coilforphoniebox/data/repository/BackupRepositoryImpl.kt +++ b/core-data/src/main/kotlin/app/coilforphoniebox/data/repository/BackupRepositoryImpl.kt @@ -9,6 +9,7 @@ import app.coilforphoniebox.domain.model.Box import app.coilforphoniebox.domain.model.Favorite import app.coilforphoniebox.domain.model.FavoriteType import app.coilforphoniebox.domain.model.FavoritesLayout +import app.coilforphoniebox.domain.model.LibraryProvider import app.coilforphoniebox.domain.model.SessionMode import app.coilforphoniebox.domain.model.ThemeMode import app.coilforphoniebox.domain.repository.BackupRepository @@ -64,6 +65,13 @@ class BackupRepositoryImpl @Inject constructor( val album: String? = null, /** Added with format version 2, together with `TRACK` favourites. */ val trackUrl: String? = null, + /** + * Which backend owns an `ALBUM` favourite, and its handle for it. Absent in a file + * written before boxes could have more than one, where the local library is what + * every album meant. + */ + val provider: String? = null, + val contentUri: String? = null, val sortIndex: Int = 0, /** * Saves the box a round of cover lookups after a reinstall. The name is a hash in @@ -79,6 +87,11 @@ class BackupRepositoryImpl @Inject constructor( val dynamicColor: Boolean = false, val sessionMode: String = SessionMode.APP_ONLY.name, val favoritesLayout: String = FavoritesLayout.GRID.name, + /** + * Defaults to false so a backup written before this setting existed imports as + * "off" — the safe reading, since the file cannot say the user ever opted in. + */ + val loadExternalCoverArt: Boolean = false, ) override suspend fun export(): String { @@ -103,6 +116,8 @@ class BackupRepositoryImpl @Inject constructor( albumArtist = favorite.albumArtist, album = favorite.album, trackUrl = favorite.trackUrl, + provider = favorite.provider, + contentUri = favorite.contentUri, sortIndex = favorite.sortIndex, coverFile = favorite.coverFile, ) @@ -114,6 +129,7 @@ class BackupRepositoryImpl @Inject constructor( dynamicColor = current.dynamicColor, sessionMode = current.sessionMode.name, favoritesLayout = current.favoritesLayout.name, + loadExternalCoverArt = current.loadExternalCoverArt, ), ) return codec.encodeToString(file) @@ -152,7 +168,10 @@ class BackupRepositoryImpl @Inject constructor( val alreadyThere = existingFavorites.any { it.type == type && it.folder == favorite.folder && it.albumArtist == favorite.albumArtist && it.album == favorite.album && - it.trackUrl == favorite.trackUrl + it.trackUrl == favorite.trackUrl && + // Two albums can share a name across backends, so the handle is part + // of what makes an imported favourite the same one. + it.contentUri == favorite.contentUri } if (alreadyThere) continue @@ -165,6 +184,8 @@ class BackupRepositoryImpl @Inject constructor( albumArtist = favorite.albumArtist, album = favorite.album, trackUrl = favorite.trackUrl, + provider = favorite.provider ?: LibraryProvider.MPD, + contentUri = favorite.contentUri, sortIndex = favorite.sortIndex, coverFile = favorite.coverFile, ).toEntity(), @@ -184,6 +205,7 @@ class BackupRepositoryImpl @Inject constructor( FavoritesLayout.entries.firstOrNull { it.name == file.settings.favoritesLayout } ?: FavoritesLayout.GRID, ) + settings.setLoadExternalCoverArt(file.settings.loadExternalCoverArt) } private val codec = Json { diff --git a/core-data/src/main/kotlin/app/coilforphoniebox/data/repository/FavoriteRepositoryImpl.kt b/core-data/src/main/kotlin/app/coilforphoniebox/data/repository/FavoriteRepositoryImpl.kt index 401a197..48ae4bc 100644 --- a/core-data/src/main/kotlin/app/coilforphoniebox/data/repository/FavoriteRepositoryImpl.kt +++ b/core-data/src/main/kotlin/app/coilforphoniebox/data/repository/FavoriteRepositoryImpl.kt @@ -26,7 +26,8 @@ class FavoriteRepositoryImpl @Inject constructor( override suspend fun matching(boxId: String, target: PlayTarget): Favorite? = when (target) { is PlayTarget.Folder -> dao.findFolder(boxId, target.path) - is PlayTarget.Album -> dao.findAlbum(boxId, target.albumArtist, target.album) + is PlayTarget.Album -> + dao.findAlbum(boxId, target.albumArtist, target.album, target.contentUri) is PlayTarget.Track -> dao.findTrack(boxId, target.url) }?.toDomain() diff --git a/core-data/src/main/kotlin/app/coilforphoniebox/data/repository/LibraryRepositoryImpl.kt b/core-data/src/main/kotlin/app/coilforphoniebox/data/repository/LibraryRepositoryImpl.kt index 36619c7..ff5ecd2 100644 --- a/core-data/src/main/kotlin/app/coilforphoniebox/data/repository/LibraryRepositoryImpl.kt +++ b/core-data/src/main/kotlin/app/coilforphoniebox/data/repository/LibraryRepositoryImpl.kt @@ -7,9 +7,11 @@ import app.coilforphoniebox.data.db.toDomain import app.coilforphoniebox.data.db.toEntity import app.coilforphoniebox.domain.model.FolderContent import app.coilforphoniebox.domain.model.LibraryAlbum +import app.coilforphoniebox.domain.model.LibraryContentType import app.coilforphoniebox.domain.model.LibraryIndexResult import app.coilforphoniebox.domain.model.LibraryIndexState import app.coilforphoniebox.domain.model.LibrarySearchResults +import app.coilforphoniebox.domain.model.LibrarySource import app.coilforphoniebox.domain.model.PlayTarget import app.coilforphoniebox.domain.model.PlaybackState import app.coilforphoniebox.domain.repository.LibraryRepository @@ -17,16 +19,20 @@ import app.coilforphoniebox.transport.Commands import app.coilforphoniebox.transport.ConnectionManager import app.coilforphoniebox.transport.LibraryParser import app.coilforphoniebox.transport.PhonieboxCommand +import app.coilforphoniebox.transport.RpcErrorException import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.update import kotlinx.coroutines.sync.Semaphore import kotlinx.coroutines.sync.withPermit +import java.util.concurrent.ConcurrentHashMap import javax.inject.Inject import javax.inject.Singleton @@ -50,6 +56,23 @@ class LibraryRepositoryImpl @Inject constructor( */ private val coverPermits = Semaphore(permits = 2) + /** + * Whether each box has the provider-neutral library surface — absent while unknown. + * + * Per box rather than global, because more than one can be configured and they need not + * run the same Phoniebox release. + */ + private val providerAware = ConcurrentHashMap() + + /** + * The backends each box reported, kept in memory rather than in Room. + * + * Derived state, cheap to ask for again, and only wanted while a screen is up — putting + * it in the database would mean another schema version for something that is refetched on + * every refresh anyway. + */ + private val sourcesByBox = MutableStateFlow>>(emptyMap()) + override fun folderContent(boxId: String, path: String): Flow = combine( dao.observeFolders(boxId, path), dao.observeTracks(boxId, path), @@ -81,34 +104,92 @@ class LibraryRepositoryImpl @Inject constructor( override fun albumsCachedAt(boxId: String): Flow = dao.observeAlbumsCachedAt(boxId) + override fun librarySources(boxId: String): Flow> = + sourcesByBox.map { it[boxId].orEmpty() }.distinctUntilChanged() + + override fun hasMultipleSources(boxId: String): Flow = + dao.observeProviderCount(boxId).map { it > 1 }.distinctUntilChanged() + override suspend fun refreshAlbums(boxId: String): Result = - transport.call(Commands.listAlbums).mapCatching { result -> + transport.call(albumListCommand(boxId)).mapCatching { result -> val now = System.currentTimeMillis() val albums = LibraryParser.albums(boxId, result, now) dao.replaceAlbums(boxId, albums.map { it.toEntity() }) } - override suspend fun ensureAlbumCover(boxId: String, albumArtist: String, album: String) { - val existing = dao.findAlbum(boxId, albumArtist, album) + /** + * `list_library_items` where the box has it, `list_albums` everywhere else. + * + * On a provider-aware box the two return the same rows today — each backend's + * implementation delegates to the other — so this is not about getting different content. + * It is about asking for only the kinds the albums tab draws, and about not asking a call + * named `list_albums` for a list that can contain playlists. + */ + private suspend fun albumListCommand(boxId: String): PhonieboxCommand = + if (ensureLibrarySources(boxId) == true) { + Commands.listLibraryItems(ALBUM_CONTENT_TYPES) + } else { + Commands.listAlbums + } + + /** + * Whether this box has the provider-neutral library surface, asking it once if unknown. + * + * `list_library_sources` is the probe for all of it: a box that answers has + * `list_library_items` and the per-entry `provider`/`content_uri` fields as well, and one + * that rejects it has none of them. So the question is asked once per box rather than a + * fallback being written into every call. + * + * Follows `PlayerRepositoryImpl.playAt`, including the part that is easy to get wrong: + * **only an error reply is an answer.** A timeout means the box is off or busy, which says + * nothing about its software, and caching "no" from that would strand a perfectly capable + * box on the old path until the app restarted. Returns null for that case, and the caller + * falls back for this attempt only — `list_albums` exists on every box, so the fallback is + * always safe. + */ + private suspend fun ensureLibrarySources(boxId: String): Boolean? { + providerAware[boxId]?.let { return it } + + val attempt = transport.call(Commands.listLibrarySources) + val verdict = probeVerdict(attempt) ?: return null + + if (verdict) { + sourcesByBox.update { it + (boxId to LibraryParser.librarySources(attempt.getOrNull())) } + } else { + Log.i(TAG, "Box has no provider-neutral library; using list_albums") + } + providerAware[boxId] = verdict + return verdict + } + + override suspend fun ensureAlbumCover(album: LibraryAlbum) { + val uri = album.contentUri.orEmpty() + val existing = dao.findAlbum(album.boxId, album.albumArtist, album.album, uri) if (existing == null || existing.coverFile != null) return coverPermits.withPermit { // Another caller may have resolved it while this one waited for a permit. - if (dao.findAlbum(boxId, albumArtist, album)?.coverFile != null) return - val coverFile = resolveCover(Commands.albumCoverArt(albumArtist, album)) ?: return - dao.setAlbumCover(boxId, albumArtist, album, coverFile) + if (dao.findAlbum(album.boxId, album.albumArtist, album.album, uri)?.coverFile != null) { + return + } + val coverFile = resolveCover(Commands.albumCoverArt(album.toPlayTarget())) ?: return + dao.setAlbumCover(album.boxId, album.albumArtist, album.album, uri, coverFile) } } override suspend fun coverFileFor(boxId: String, target: PlayTarget): String? = when (target) { - is PlayTarget.Album -> - dao.findAlbum(boxId, target.albumArtist, target.album)?.coverFile + is PlayTarget.Album -> { + val uri = target.contentUri.orEmpty() + dao.findAlbum(boxId, target.albumArtist, target.album, uri)?.coverFile ?: coverPermits.withPermit { - resolveCover(Commands.albumCoverArt(target.albumArtist, target.album)) + resolveCover(Commands.albumCoverArt(target)) // The album grid asks the same question, so answer it here too. A // missing row makes this a no-op rather than an error. - ?.also { dao.setAlbumCover(boxId, target.albumArtist, target.album, it) } + ?.also { + dao.setAlbumCover(boxId, target.albumArtist, target.album, uri, it) + } } + } is PlayTarget.Track -> coverPermits.withPermit { resolveCover(Commands.singleCoverArt(target.url)) } @@ -150,7 +231,7 @@ class LibraryRepositoryImpl @Inject constructor( val payload = transport.call(command).getOrNull() ?: return null when (val art = LibraryParser.coverArt(payload)) { - is LibraryParser.CoverArt.Available -> return art.fileName + is LibraryParser.CoverArt.Available -> return art.coverRef // Nothing to show, and nothing to gain from asking again. LibraryParser.CoverArt.Missing -> return null @@ -270,6 +351,17 @@ class LibraryRepositoryImpl @Inject constructor( private companion object { const val TAG = "CoilLibrary" + + /** + * The kinds the albums tab can draw. Tracks are left out deliberately: a streaming + * account's saved songs run to thousands of entries, and the tab shows one tile each. + */ + val ALBUM_CONTENT_TYPES = listOf( + LibraryContentType.ALBUM, + LibraryContentType.PLAYLIST, + LibraryContentType.COLLECTION, + ) + const val COVER_ATTEMPTS = 3 const val COVER_RETRY_MILLIS = 1_000L const val RESCAN_SETTLE_MILLIS = 3_000L diff --git a/core-data/src/main/kotlin/app/coilforphoniebox/data/repository/PlayerRepositoryImpl.kt b/core-data/src/main/kotlin/app/coilforphoniebox/data/repository/PlayerRepositoryImpl.kt index 314dd3d..60d7a69 100644 --- a/core-data/src/main/kotlin/app/coilforphoniebox/data/repository/PlayerRepositoryImpl.kt +++ b/core-data/src/main/kotlin/app/coilforphoniebox/data/repository/PlayerRepositoryImpl.kt @@ -9,6 +9,7 @@ import app.coilforphoniebox.domain.model.QueueEntry import app.coilforphoniebox.domain.model.RepeatMode import app.coilforphoniebox.domain.model.SleepTimerStatus import app.coilforphoniebox.domain.model.VolumeStatus +import app.coilforphoniebox.data.settings.SettingsStore import app.coilforphoniebox.domain.repository.PlayerRepository import app.coilforphoniebox.transport.Commands import app.coilforphoniebox.transport.ConnectionManager @@ -23,12 +24,14 @@ import kotlinx.coroutines.NonCancellable import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock @@ -48,6 +51,7 @@ import kotlin.math.abs @Singleton class PlayerRepositoryImpl @Inject constructor( private val transport: ConnectionManager, + private val settings: SettingsStore, @TransportScope private val scope: CoroutineScope, ) : PlayerRepository { @@ -66,14 +70,13 @@ class PlayerRepositoryImpl @Inject constructor( /** Songs the box has told us have no artwork, so they are not asked about again. */ private val songsWithoutArt: MutableSet = ConcurrentHashMap.newKeySet() - private val _coverUrl = MutableStateFlow(null) - /** - * The same cover as [_coverUrl], as the bare file name. + * The resolved cover reference for the current song, as the box gave it: a name in its + * cover cache, or an absolute URL when the backend serves its own artwork. * - * Kept beside the URL rather than derived from it: a favourite stores the file name, and - * picking it back out of a URL would make [Box.coverUrl] a format two places have to - * agree on. + * Held rather than derived from the URL, because a favourite stores this and picking it + * back out of a URL would make [Box.coverUrl]'s output a format two places have to agree + * on. */ private val _coverFile = MutableStateFlow(null) @@ -87,8 +90,18 @@ class PlayerRepositoryImpl @Inject constructor( * controls included — for as long as the lookup takes, or forever if it keeps being * restarted. Cover art is the least important thing on the screen; it must never be * able to hold up the rest. + * + * Derived from the resolved reference rather than written beside it, so that turning + * [AppSettings.loadExternalCoverArt] on or off re-renders the cover already in hand + * instead of waiting for the next song to come round. */ - override val coverUrl: StateFlow = _coverUrl.asStateFlow() + override val coverUrl: StateFlow = combine( + _coverFile, + transport.activeBox, + settings.settings.map { it.loadExternalCoverArt }.distinctUntilChanged(), + ) { coverRef, box, allowExternal -> + coverRef?.let { box?.coverUrl(it, allowExternal) } + }.stateIn(scope, SharingStarted.Eagerly, null) /** * Set for as long as a lookup is outstanding, so a null [coverUrl] can be read as "no @@ -231,16 +244,13 @@ class PlayerRepositoryImpl @Inject constructor( if (key != lastKey) { // Showing the previous song's artwork would be worse than showing none. _coverFile.value = null - _coverUrl.value = null lastKey = key } // An idle player is not a lookup in progress. Saying otherwise would leave // the screen on a placeholder waiting for an answer that is not coming. _coverPending.value = file != null && box != null if (file != null && box != null) { - val coverFile = resolveCoverFile(file, box) - _coverFile.value = coverFile - _coverUrl.value = coverFile?.let { box.coverUrl(it) } + _coverFile.value = resolveCoverFile(file, box) // Answered, with or without a cover — either way the wait is over. A // lookup abandoned by `collectLatest` never gets here, which is correct: // the song that replaced it sets this again on its own way through. @@ -265,8 +275,8 @@ class PlayerRepositoryImpl @Inject constructor( val payload = transport.call(Commands.singleCoverArt(file)).getOrNull() ?: return null when (val art = LibraryParser.coverArt(payload)) { is LibraryParser.CoverArt.Available -> { - resolvedCovers[key] = art.fileName - return art.fileName + resolvedCovers[key] = art.coverRef + return art.coverRef } LibraryParser.CoverArt.Missing -> { diff --git a/core-data/src/main/kotlin/app/coilforphoniebox/data/repository/ProbeVerdict.kt b/core-data/src/main/kotlin/app/coilforphoniebox/data/repository/ProbeVerdict.kt new file mode 100644 index 0000000..9bc2d4c --- /dev/null +++ b/core-data/src/main/kotlin/app/coilforphoniebox/data/repository/ProbeVerdict.kt @@ -0,0 +1,25 @@ +package app.coilforphoniebox.data.repository + +import app.coilforphoniebox.transport.RpcErrorException + +/** + * What one attempt at an optional RPC says about whether the box supports it. + * + * `true` supported, `false` not, **null still unknown** — and the null is the whole reason + * this is a function rather than two lines at the call site. + * + * A box answers an unknown method by raising before the plugin body runs, and + * `jukebox/rpc/server.py` turns that into an error reply. So an error reply is a real answer, + * and a cheap one: nothing happened on the box. Every *other* failure is not an answer at + * all. A timeout, a torn-down socket or an unreachable host says the box is off or busy, + * which says nothing whatever about what its software can do — and recording "unsupported" + * from one of those would strand a perfectly capable box on the fallback path until the app + * was restarted. + * + * The same distinction as `PlayerRepositoryImpl.playAt`, which probes for `play(pos=…)`. + */ +internal fun probeVerdict(attempt: Result<*>): Boolean? = when { + attempt.isSuccess -> true + attempt.exceptionOrNull() is RpcErrorException -> false + else -> null +} diff --git a/core-data/src/main/kotlin/app/coilforphoniebox/data/repository/SettingsRepositoryImpl.kt b/core-data/src/main/kotlin/app/coilforphoniebox/data/repository/SettingsRepositoryImpl.kt index 820e05a..abbea6a 100644 --- a/core-data/src/main/kotlin/app/coilforphoniebox/data/repository/SettingsRepositoryImpl.kt +++ b/core-data/src/main/kotlin/app/coilforphoniebox/data/repository/SettingsRepositoryImpl.kt @@ -28,6 +28,9 @@ class SettingsRepositoryImpl @Inject constructor( override suspend fun setFavoritesLayout(layout: FavoritesLayout) = store.setFavoritesLayout(layout) + override suspend fun setLoadExternalCoverArt(enabled: Boolean) = + store.setLoadExternalCoverArt(enabled) + override suspend fun setOnboardingComplete(complete: Boolean) = store.setOnboardingComplete(complete) } diff --git a/core-data/src/main/kotlin/app/coilforphoniebox/data/settings/SettingsStore.kt b/core-data/src/main/kotlin/app/coilforphoniebox/data/settings/SettingsStore.kt index e65ab63..f679009 100644 --- a/core-data/src/main/kotlin/app/coilforphoniebox/data/settings/SettingsStore.kt +++ b/core-data/src/main/kotlin/app/coilforphoniebox/data/settings/SettingsStore.kt @@ -29,6 +29,7 @@ class SettingsStore @Inject constructor( dynamicColor = prefs[DYNAMIC_COLOR] ?: false, sessionMode = prefs[SESSION_MODE].toSessionMode(), favoritesLayout = prefs[FAVORITES_LAYOUT].toFavoritesLayout(), + loadExternalCoverArt = prefs[LOAD_EXTERNAL_COVER_ART] ?: false, activeBoxId = prefs[ACTIVE_BOX_ID], onboardingComplete = prefs[ONBOARDING_COMPLETE] ?: false, ) @@ -44,6 +45,8 @@ class SettingsStore @Inject constructor( suspend fun setFavoritesLayout(layout: FavoritesLayout) = put(FAVORITES_LAYOUT, layout.name) + suspend fun setLoadExternalCoverArt(enabled: Boolean) = put(LOAD_EXTERNAL_COVER_ART, enabled) + suspend fun setActiveBoxId(boxId: String?) { dataStore.edit { prefs -> if (boxId == null) prefs.remove(ACTIVE_BOX_ID) else prefs[ACTIVE_BOX_ID] = boxId @@ -71,6 +74,7 @@ class SettingsStore @Inject constructor( val DYNAMIC_COLOR = booleanPreferencesKey("dynamic_color") val SESSION_MODE = stringPreferencesKey("session_mode") val FAVORITES_LAYOUT = stringPreferencesKey("favorites_layout") + val LOAD_EXTERNAL_COVER_ART = booleanPreferencesKey("load_external_cover_art") val ACTIVE_BOX_ID = stringPreferencesKey("active_box_id") val ONBOARDING_COMPLETE = booleanPreferencesKey("onboarding_complete") } diff --git a/core-data/src/test/kotlin/app/coilforphoniebox/data/db/MigrationTest.kt b/core-data/src/test/kotlin/app/coilforphoniebox/data/db/MigrationTest.kt new file mode 100644 index 0000000..a15ce35 --- /dev/null +++ b/core-data/src/test/kotlin/app/coilforphoniebox/data/db/MigrationTest.kt @@ -0,0 +1,156 @@ +package app.coilforphoniebox.data.db + +import androidx.room.testing.MigrationTestHelper +import androidx.test.platform.app.InstrumentationRegistry +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +/** + * Every schema change is migrated rather than dropped, and this is where that claim is + * checked against a real SQLite file instead of being reasoned about. + * + * The stakes are lopsided. A box row and a favourite are the only things in this database a + * user cannot get back by pointing Coil at the box again, so a migration that loses one is a + * bug nobody can undo. The library cache is the opposite — disposable by design — but it is + * still not thrown away where it does not have to be. + */ +@RunWith(RobolectricTestRunner::class) +class MigrationTest { + + @get:Rule + val helper = MigrationTestHelper( + InstrumentationRegistry.getInstrumentation(), + CoilDatabase::class.java, + ) + + /** + * Version 4 re-keys `library_albums` on the content URI, which SQLite cannot do in place, + * so the table is rebuilt. Rebuilding is where rows get lost, hence the fixture: an album + * with a resolved cover, and a favourite, both of which must come through untouched. + */ + @Test + fun `migrating to 4 keeps every album and its cover`() { + helper.createDatabase(DB, 3).use { db -> + db.execSQL( + "INSERT INTO boxes (id, displayName, host, rpcPort, pubPort, addedAt, " + + "autoSessionEnabled, networkSsid, lastSeenAt, sortIndex) " + + "VALUES ('box-1', 'Living room', 'phoniebox.local', 5555, 5558, 0, 0, NULL, NULL, 0)", + ) + db.execSQL( + "INSERT INTO library_albums (boxId, albumArtist, album, coverFile, searchText, cachedAt) " + + "VALUES ('box-1', 'Bibi', 'Hexerei', 'a1.jpg', 'bibi hexerei', 42)", + ) + db.execSQL( + "INSERT INTO library_albums (boxId, albumArtist, album, coverFile, searchText, cachedAt) " + + "VALUES ('box-1', 'Benjamin', 'Zoo', NULL, 'benjamin zoo', 42)", + ) + } + + val db = helper.runMigrationsAndValidate(DB, 4, true, MIGRATION_3_4) + + db.query( + "SELECT albumArtist, album, coverFile, cachedAt, contentUri, provider, contentType " + + "FROM library_albums ORDER BY albumArtist", + ).use { cursor -> + assertEquals("both albums survive the rebuild", 2, cursor.count) + + cursor.moveToFirst() + assertEquals("Benjamin", cursor.getString(0)) + assertEquals("Zoo", cursor.getString(1)) + assertNull(cursor.getString(2)) + + cursor.moveToNext() + assertEquals("Bibi", cursor.getString(0)) + assertEquals("Hexerei", cursor.getString(1)) + // The cover is the point: `replaceAlbums` goes to some trouble to keep resolved + // covers, and dropping the table would have thrown them all away. + assertEquals("a1.jpg", cursor.getString(2)) + assertEquals(42, cursor.getInt(3)) + // Not fallback defaults — every pre-existing row genuinely came from a box with + // MPD as its only backend. + assertEquals("", cursor.getString(4)) + assertEquals("mpd", cursor.getString(5)) + assertEquals("album", cursor.getString(6)) + } + } + + /** + * `favorites` is additive, the way `trackUrl` arrived in version 2. Null is the honest + * value for a row saved before a box could have a second backend, and the mapper reads it + * back as MPD. + */ + @Test + fun `migrating to 4 keeps favourites and leaves their new columns null`() { + helper.createDatabase(DB, 3).use { db -> + db.execSQL( + "INSERT INTO boxes (id, displayName, host, rpcPort, pubPort, addedAt, " + + "autoSessionEnabled, networkSsid, lastSeenAt, sortIndex) " + + "VALUES ('box-1', 'Living room', 'phoniebox.local', 5555, 5558, 0, 0, NULL, NULL, 0)", + ) + db.execSQL( + "INSERT INTO favorites (boxId, label, type, folder, albumArtist, album, " + + "trackUrl, coverFile, sortIndex, launchCount, shortcutPinned) " + + "VALUES ('box-1', 'Hexerei', 'ALBUM', NULL, 'Bibi', 'Hexerei', NULL, 'a1.jpg', 3, 7, 1)", + ) + } + + val db = helper.runMigrationsAndValidate(DB, 4, true, MIGRATION_3_4) + + db.query( + "SELECT label, albumArtist, album, coverFile, sortIndex, launchCount, " + + "shortcutPinned, provider, contentUri FROM favorites", + ).use { cursor -> + assertEquals(1, cursor.count) + cursor.moveToFirst() + assertEquals("Hexerei", cursor.getString(0)) + assertEquals("Bibi", cursor.getString(1)) + assertEquals("Hexerei", cursor.getString(2)) + assertEquals("a1.jpg", cursor.getString(3)) + // Everything a user set by hand comes through: order, launch count, whether it + // is pinned to the home screen. + assertEquals(3, cursor.getInt(4)) + assertEquals(7, cursor.getInt(5)) + assertEquals(1, cursor.getInt(6)) + assertTrue("provider is unset, not guessed", cursor.isNull(7)) + assertTrue("content URI is unset, not guessed", cursor.isNull(8)) + } + } + + /** + * The whole chain, because a user on the very first release migrates through all of it in + * one go and the intermediate steps are the ones nobody runs by hand. + */ + @Test + fun `a version 1 database migrates all the way to 4`() { + helper.createDatabase(DB, 1).use { db -> + db.execSQL( + "INSERT INTO boxes (id, displayName, host, rpcPort, pubPort, addedAt, " + + "autoSessionEnabled, networkSsid, lastSeenAt, sortIndex) " + + "VALUES ('box-1', 'Living room', 'phoniebox.local', 5555, 5558, 0, 0, NULL, NULL, 0)", + ) + db.execSQL( + "INSERT INTO favorites (boxId, label, type, folder, albumArtist, album, " + + "coverFile, sortIndex, launchCount, shortcutPinned) " + + "VALUES ('box-1', 'Bibi', 'FOLDER', 'Audiobooks/Bibi', NULL, NULL, NULL, 0, 0, 0)", + ) + } + + val db = helper.runMigrationsAndValidate(DB, 4, true, *ALL_MIGRATIONS) + + db.query("SELECT label, folder FROM favorites").use { cursor -> + assertEquals("the favourite survives all three migrations", 1, cursor.count) + cursor.moveToFirst() + assertEquals("Bibi", cursor.getString(0)) + assertEquals("Audiobooks/Bibi", cursor.getString(1)) + } + } + + private companion object { + const val DB = "migration-test" + } +} diff --git a/core-data/src/test/kotlin/app/coilforphoniebox/data/repository/ProbeVerdictTest.kt b/core-data/src/test/kotlin/app/coilforphoniebox/data/repository/ProbeVerdictTest.kt new file mode 100644 index 0000000..23e49cb --- /dev/null +++ b/core-data/src/test/kotlin/app/coilforphoniebox/data/repository/ProbeVerdictTest.kt @@ -0,0 +1,39 @@ +package app.coilforphoniebox.data.repository + +import app.coilforphoniebox.transport.NotConnectedException +import app.coilforphoniebox.transport.RpcErrorException +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test +import java.io.IOException +import java.util.concurrent.TimeoutException + +class ProbeVerdictTest { + + @Test + fun `an answer means the box has the call`() { + assertEquals(true, probeVerdict(Result.success(Unit))) + } + + /** + * The box raises before the plugin body runs and the RPC server formats that as an error + * reply, so this is a real answer and a free one — nothing happened on the box. + */ + @Test + fun `an error reply means the box does not have the call`() { + val reply = RpcErrorException("player.ctrl.list_library_sources", "no such method") + assertEquals(false, probeVerdict(Result.failure(reply))) + } + + /** + * The case worth having a test for. A box that is switched off, busy or behind a dropped + * socket has told us nothing about its software. Reading any of these as "unsupported" + * would pin a capable box to the fallback path for the rest of the session. + */ + @Test + fun `every other failure leaves the question open`() { + assertNull(probeVerdict(Result.failure(TimeoutException()))) + assertNull(probeVerdict(Result.failure(NotConnectedException()))) + assertNull(probeVerdict(Result.failure(IOException("socket closed")))) + } +} diff --git a/core-data/src/test/resources/robolectric.properties b/core-data/src/test/resources/robolectric.properties new file mode 100644 index 0000000..a9252b7 --- /dev/null +++ b/core-data/src/test/resources/robolectric.properties @@ -0,0 +1,4 @@ +# One below targetSdk, for the same reason as the app module's copy: Robolectric's SDK 36 image +# needs a newer JDK than the Temurin 17 every workflow here pins, so it would pass locally and +# throw on CI. SDK 35 runs on 17, which makes the two the same platform. +sdk=35 diff --git a/core-domain/src/main/kotlin/app/coilforphoniebox/domain/model/AppSettings.kt b/core-domain/src/main/kotlin/app/coilforphoniebox/domain/model/AppSettings.kt index 6b0143b..d3f1751 100644 --- a/core-domain/src/main/kotlin/app/coilforphoniebox/domain/model/AppSettings.kt +++ b/core-domain/src/main/kotlin/app/coilforphoniebox/domain/model/AppSettings.kt @@ -29,6 +29,16 @@ data class AppSettings( val dynamicColor: Boolean = false, val sessionMode: SessionMode = SessionMode.APP_ONLY, val favoritesLayout: FavoritesLayout = FavoritesLayout.GRID, + /** + * Whether cover art may be fetched from somewhere other than the box. + * + * A provider-neutral box can answer a cover request with an absolute URL belonging to + * the backend the content came from — Spotify hands back `https://i.scdn.co/…` rather + * than a name in the box's own cache. Loading one means the phone talks to a third + * party, which is the one thing §16 and the Data Safety declaration promise it does + * not do, so it stays off until the user says otherwise. + */ + val loadExternalCoverArt: Boolean = false, val activeBoxId: String? = null, val onboardingComplete: Boolean = false, ) diff --git a/core-domain/src/main/kotlin/app/coilforphoniebox/domain/model/Box.kt b/core-domain/src/main/kotlin/app/coilforphoniebox/domain/model/Box.kt index 8a5d2de..f2ecb63 100644 --- a/core-domain/src/main/kotlin/app/coilforphoniebox/domain/model/Box.kt +++ b/core-domain/src/main/kotlin/app/coilforphoniebox/domain/model/Box.kt @@ -22,11 +22,25 @@ data class Box( val sortIndex: Int = 0, ) { /** - * Cover art comes over plain HTTP from the box's web server, not over ZMQ. - * `get_single_coverart` returns a bare filename that is appended here. + * Where to load a cover from, or null for "do not load this one". + * + * Cover art comes over plain HTTP from the box's web server, not over ZMQ, and + * `get_single_coverart` normally answers with a bare filename in the box's own cache + * that is appended here. + * + * A provider-neutral box can answer with an **absolute URL** instead, belonging to + * whichever backend the content came from — Spotify hands back `https://i.scdn.co/…`. + * Appending that to the box's address would produce a URL that 404s, so it is passed + * through unchanged — but only when [allowExternal] says so. Loading it means the phone + * talks to a third party, which is the one thing the LAN-only design promises it does + * not do (§16), so [AppSettings.loadExternalCoverArt] is off until the user opts in and + * a refused cover falls back to the same placeholder as a missing one. */ - fun coverUrl(coverFile: String): String = - "http://$host/cover-cache/${coverFile.trimStart('/')}" + fun coverUrl(coverRef: String, allowExternal: Boolean): String? = when { + !coverRef.isExternalCoverRef() -> "http://$host/cover-cache/${coverRef.trimStart('/')}" + allowExternal -> coverRef + else -> null + } companion object { /** TCP, not the WebSocket ports — JeroMQ has no `ws://` transport. */ @@ -34,3 +48,17 @@ data class Box( const val DEFAULT_PUB_PORT = 5558 } } + +/** + * Whether a cover reference points somewhere other than the active box's cover cache. + * + * Deliberately a prefix test and not a URL parse: the only thing that distinguishes the two + * kinds of answer is that one is a bare cache filename and the other is an absolute + * `http(s)` URL, and a filename can contain anything else a parser might trip over. + * + * Lives here rather than next to its two callers because both the parser that reads the + * box's answer and the model that turns it into a URL have to agree on the same test — + * disagreeing would either mangle an external URL or hand a filename to a third party. + */ +fun String.isExternalCoverRef(): Boolean = + startsWith("http://", ignoreCase = true) || startsWith("https://", ignoreCase = true) diff --git a/core-domain/src/main/kotlin/app/coilforphoniebox/domain/model/Favorite.kt b/core-domain/src/main/kotlin/app/coilforphoniebox/domain/model/Favorite.kt index 6c2056f..75a6be2 100644 --- a/core-domain/src/main/kotlin/app/coilforphoniebox/domain/model/Favorite.kt +++ b/core-domain/src/main/kotlin/app/coilforphoniebox/domain/model/Favorite.kt @@ -17,6 +17,15 @@ data class Favorite( val album: String? = null, /** MPD URL of a single file, for a TRACK favourite. */ val trackUrl: String? = null, + /** + * Which backend owns an ALBUM favourite, and that backend's own handle for it. + * + * Saved so the favourite still starts the right thing on a box with more than one + * backend. A row from before this existed reads as [LibraryProvider.MPD] with no URI, + * which is exactly what it was. + */ + val provider: String = LibraryProvider.MPD, + val contentUri: String? = null, val coverFile: String? = null, val sortIndex: Int = 0, val launchCount: Int = 0, @@ -30,7 +39,11 @@ data class Favorite( fun toPlayTarget(): PlayTarget? = when (type) { FavoriteType.FOLDER -> folder?.let { PlayTarget.Folder(it) } FavoriteType.ALBUM -> - if (albumArtist != null && album != null) PlayTarget.Album(albumArtist, album) else null + if (albumArtist != null && album != null) { + PlayTarget.Album(albumArtist, album, provider, contentUri) + } else { + null + } FavoriteType.TRACK -> trackUrl?.let { PlayTarget.Track(it) } } @@ -53,6 +66,8 @@ data class Favorite( type = FavoriteType.ALBUM, albumArtist = target.albumArtist, album = target.album, + provider = target.provider, + contentUri = target.contentUri, coverFile = coverFile, ) diff --git a/core-domain/src/main/kotlin/app/coilforphoniebox/domain/model/Library.kt b/core-domain/src/main/kotlin/app/coilforphoniebox/domain/model/Library.kt index 9a54539..76b8992 100644 --- a/core-domain/src/main/kotlin/app/coilforphoniebox/domain/model/Library.kt +++ b/core-domain/src/main/kotlin/app/coilforphoniebox/domain/model/Library.kt @@ -31,13 +31,87 @@ data class LibraryTrack( get() = title?.takeIf { it.isNotBlank() } ?: url.substringAfterLast('/') } +/** + * Which backend on the box an item came from. + * + * Deliberately a plain string rather than an enum: a box registers its backends by name at + * startup, so the set is open — a Phoniebox could gain a third one Coil has never heard of, + * and the right thing to do with an unknown name is to hand it back to the box unchanged + * rather than fail to model it. [MPD] is the only one Coil can assume, because it is the + * default backend and the only one a box before the provider-neutral player had at all. + */ +object LibraryProvider { + const val MPD = "mpd" +} + +/** + * What kind of thing a library entry is. + * + * A provider-neutral box returns playlists and saved-track collections through the same call + * as albums, so "album" is no longer the only answer. Open for the same reason as + * [LibraryProvider], and [ALBUM] is what an entry that does not say is taken to be. + */ +object LibraryContentType { + const val ALBUM = "album" + const val PLAYLIST = "playlist" + const val COLLECTION = "collection" + const val TRACK = "track" +} + +/** + * A backend the box browses with, as the box describes itself. + * + * The only authoritative account of which backends exist and what to call them. Working it + * out from the entries that came back instead would be wrong in the case that matters: the + * box swallows a backend whose catalogue failed, so a streaming account that is merely + * unreachable would look like a box that never had one. + * + * [label] is the box's own English word for it — "Local", "Spotify" — and is a last resort. + * Coil translates the backends it knows and falls back to this for one it does not. + */ +data class LibrarySource( + val id: String, + val label: String, +) + +/** + * One entry in the albums list — which, on a box with more than one backend, is not always + * an album: see [contentType]. + * + * **Identity is all four of [boxId], [albumArtist], [album] and [contentUri]**, not the + * artist and album alone. Once a box has two backends the same pair can name two different + * things, and telling them apart is the difference between playing the right one and + * clearing the queue to play nothing. + */ data class LibraryAlbum( val boxId: String, val albumArtist: String, val album: String, val coverFile: String? = null, val cachedAt: Long = 0L, -) + val provider: String = LibraryProvider.MPD, + /** + * The backend's own handle for this entry — `spotify:album:…` and the like. Null for + * anything MPD holds, which is addressed by artist and album instead, and null for every + * entry from a box predating the provider-neutral player. + */ + val contentUri: String? = null, + val contentType: String = LibraryContentType.ALBUM, +) { + /** + * The play target for this entry, carrying the provider and content URI with it. + * + * Use this rather than building a [PlayTarget.Album] from the artist and album by hand: + * dropping the other two is exactly what makes a Spotify album clear the queue and play + * nothing. + */ + fun toPlayTarget() = PlayTarget.Album( + albumArtist = albumArtist, + album = album, + provider = provider, + contentUri = contentUri, + ) +} /** * What a search over the cached library found. diff --git a/core-domain/src/main/kotlin/app/coilforphoniebox/domain/model/PlayTarget.kt b/core-domain/src/main/kotlin/app/coilforphoniebox/domain/model/PlayTarget.kt index 16fccb0..ff80279 100644 --- a/core-domain/src/main/kotlin/app/coilforphoniebox/domain/model/PlayTarget.kt +++ b/core-domain/src/main/kotlin/app/coilforphoniebox/domain/model/PlayTarget.kt @@ -4,8 +4,38 @@ package app.coilforphoniebox.domain.model sealed interface PlayTarget { data class Folder(val path: String) : PlayTarget - data class Album(val albumArtist: String, val album: String) : PlayTarget + /** + * An album, playlist or saved-track collection — whatever the albums list offered. + * + * [provider] and [contentUri] say *which* backend on the box owns it. They default to + * MPD and null, which is both what a box with a single backend always means and what a + * favourite or shortcut created before either field existed can supply — so the defaults + * are the compatible answer rather than a guess. They go on the wire only when they say + * something, because a box predating the provider-neutral player rejects the kwargs + * outright. + */ + data class Album( + val albumArtist: String, + val album: String, + val provider: String = LibraryProvider.MPD, + val contentUri: String? = null, + ) : PlayTarget /** A single file, by MPD URL. */ data class Track(val url: String) : PlayTarget } + +/** + * A stable string identifying this target, for list keys and favourite lookups. + * + * One definition rather than the same interpolation written at each use, because the album + * case has to include the content URI and getting that wrong is not a subtle failure: two + * sources offering the same record produce two entries, and a key built from the artist and + * album alone makes them collide — which a `LazyVerticalGrid` answers by throwing. + */ +val PlayTarget.key: String + get() = when (this) { + is PlayTarget.Folder -> "folder:$path" + is PlayTarget.Album -> "album:$albumArtist/$album/${contentUri.orEmpty()}" + is PlayTarget.Track -> "track:$url" + } diff --git a/core-domain/src/main/kotlin/app/coilforphoniebox/domain/repository/LibraryRepository.kt b/core-domain/src/main/kotlin/app/coilforphoniebox/domain/repository/LibraryRepository.kt index 368bcf6..27011a4 100644 --- a/core-domain/src/main/kotlin/app/coilforphoniebox/domain/repository/LibraryRepository.kt +++ b/core-domain/src/main/kotlin/app/coilforphoniebox/domain/repository/LibraryRepository.kt @@ -5,6 +5,7 @@ import app.coilforphoniebox.domain.model.LibraryAlbum import app.coilforphoniebox.domain.model.LibraryIndexResult import app.coilforphoniebox.domain.model.LibraryIndexState import app.coilforphoniebox.domain.model.LibrarySearchResults +import app.coilforphoniebox.domain.model.LibrarySource import app.coilforphoniebox.domain.model.PlayTarget import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.StateFlow @@ -25,6 +26,25 @@ interface LibraryRepository { /** Most recent successful album refresh, for the freshness hint. */ fun albumsCachedAt(boxId: String): Flow + /** + * The backends this box browses with, as it described them; empty until a refresh has + * asked, and empty for a box that has no such notion. + * + * Only good for *naming* a backend. Whether a box has more than one is answered from the + * cached albums instead, which survive a cold start where this does not. + */ + fun librarySources(boxId: String): Flow> + + /** + * Whether this box's albums came from more than one backend. + * + * Read from the cached albums rather than from [librarySources], so it is already right + * on a cold start — the sources are only known once a refresh has asked, and a UI that + * flickered from "one source" to "two" a second after opening would be worse than one + * that never mentioned sources at all. + */ + fun hasMultipleSources(boxId: String): Flow + suspend fun refreshAlbums(boxId: String): Result /** @@ -32,8 +52,12 @@ interface LibraryRepository { * * Deliberately per album and on demand: asking for every cover during a refresh * would be one RPC per album on a socket the box also uses for card detection. + * + * Takes the whole album rather than its parts, because the lookup has to be routed and + * stored against the full identity — artist and album alone address a different row on a + * box with more than one backend. */ - suspend fun ensureAlbumCover(boxId: String, albumArtist: String, album: String) + suspend fun ensureAlbumCover(album: LibraryAlbum) /** * Cover file name for [target], from the cache if it is there and from the box if not. diff --git a/core-domain/src/main/kotlin/app/coilforphoniebox/domain/repository/SettingsRepository.kt b/core-domain/src/main/kotlin/app/coilforphoniebox/domain/repository/SettingsRepository.kt index 8ca8dff..29796b8 100644 --- a/core-domain/src/main/kotlin/app/coilforphoniebox/domain/repository/SettingsRepository.kt +++ b/core-domain/src/main/kotlin/app/coilforphoniebox/domain/repository/SettingsRepository.kt @@ -19,5 +19,7 @@ interface SettingsRepository { suspend fun setFavoritesLayout(layout: FavoritesLayout) + suspend fun setLoadExternalCoverArt(enabled: Boolean) + suspend fun setOnboardingComplete(complete: Boolean) } diff --git a/core-domain/src/test/kotlin/app/coilforphoniebox/domain/model/BoxTest.kt b/core-domain/src/test/kotlin/app/coilforphoniebox/domain/model/BoxTest.kt index d10f4c3..29c863f 100644 --- a/core-domain/src/test/kotlin/app/coilforphoniebox/domain/model/BoxTest.kt +++ b/core-domain/src/test/kotlin/app/coilforphoniebox/domain/model/BoxTest.kt @@ -1,6 +1,7 @@ package app.coilforphoniebox.domain.model import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull import org.junit.Test class BoxTest { @@ -22,7 +23,56 @@ class BoxTest { /** Cover art comes over HTTP from the box's web server, not over ZMQ. */ @Test fun `cover urls point at the box's cover cache`() { - assertEquals("http://phoniebox.local/cover-cache/a1.jpg", box.coverUrl("a1.jpg")) - assertEquals("http://phoniebox.local/cover-cache/a1.jpg", box.coverUrl("/a1.jpg")) + assertEquals( + "http://phoniebox.local/cover-cache/a1.jpg", + box.coverUrl("a1.jpg", allowExternal = false), + ) + assertEquals( + "http://phoniebox.local/cover-cache/a1.jpg", + box.coverUrl("/a1.jpg", allowExternal = false), + ) + } + + /** + * A cache name is the box's own artwork, so the setting has nothing to say about it — + * turning external covers on must not change where a local cover is loaded from. + */ + @Test + fun `a cache name is unaffected by the external setting`() { + assertEquals( + "http://phoniebox.local/cover-cache/a1.jpg", + box.coverUrl("a1.jpg", allowExternal = true), + ) + } + + /** + * An absolute URL belongs to whichever backend served the content — Spotify answers with + * one. Appending it to the box's address would 404, so it is passed through whole. + */ + @Test + fun `an absolute url is passed through when external covers are allowed`() { + assertEquals( + "https://i.scdn.co/image/ab67616d", + box.coverUrl("https://i.scdn.co/image/ab67616d", allowExternal = true), + ) + assertEquals( + "http://cdn.example/art.jpg", + box.coverUrl("http://cdn.example/art.jpg", allowExternal = true), + ) + } + + /** + * The LAN-only promise (§16): with the setting off, an external cover is not loaded at + * all rather than being loaded from the wrong place. + */ + @Test + fun `an absolute url is refused when external covers are not allowed`() { + assertNull(box.coverUrl("https://i.scdn.co/image/ab67616d", allowExternal = false)) + } + + /** Scheme matching is case-insensitive, so `HTTPS://` cannot smuggle a URL through. */ + @Test + fun `external detection ignores scheme case`() { + assertNull(box.coverUrl("HTTPS://i.scdn.co/image/ab67616d", allowExternal = false)) } } diff --git a/core-transport/src/main/kotlin/app/coilforphoniebox/transport/LibraryParser.kt b/core-transport/src/main/kotlin/app/coilforphoniebox/transport/LibraryParser.kt index 8689769..f5ea48a 100644 --- a/core-transport/src/main/kotlin/app/coilforphoniebox/transport/LibraryParser.kt +++ b/core-transport/src/main/kotlin/app/coilforphoniebox/transport/LibraryParser.kt @@ -2,8 +2,12 @@ package app.coilforphoniebox.transport import app.coilforphoniebox.domain.model.FolderContent import app.coilforphoniebox.domain.model.LibraryAlbum +import app.coilforphoniebox.domain.model.LibraryContentType +import app.coilforphoniebox.domain.model.LibraryProvider +import app.coilforphoniebox.domain.model.LibrarySource import app.coilforphoniebox.domain.model.LibraryFolder import app.coilforphoniebox.domain.model.LibraryTrack +import app.coilforphoniebox.domain.model.isExternalCoverRef import kotlinx.serialization.json.JsonArray import kotlinx.serialization.json.JsonElement import kotlinx.serialization.json.JsonObject @@ -74,6 +78,15 @@ object LibraryParser { /** * `list_albums` is MPD's `list album group albumartist`, so each entry carries one * album artist and either a single album or an array of them. + * + * A provider-neutral box adds `provider`, `content_uri` and `content_type` to every + * entry — MPD's own included — and, once the box has more than one backend, returns all + * of their catalogues concatenated. Those three fields are then the only thing telling + * two entries apart, so they are read here and kept: **dropping them is what makes + * `play_album` fall through to MPD, find nothing, and clear the queue.** + * + * A box without the provider-neutral player sends none of them, which reads as MPD, no + * content URI, and an album — exactly what such a box only ever has. */ fun albums(boxId: String, result: JsonElement?, cachedAt: Long): List { val entries = (result as? JsonArray).orEmpty() @@ -85,10 +98,26 @@ object LibraryParser { val artist = element.string("albumartist") ?: element.string("artist") ?: "" + val provider = element.string("provider") + ?.takeIf { it.isNotBlank() } + ?: LibraryProvider.MPD + val contentUri = element.string("content_uri")?.takeIf { it.isNotBlank() } + val contentType = element.string("content_type") + ?.takeIf { it.isNotBlank() } + ?: LibraryContentType.ALBUM + element["album"].asStringList() .filter { it.isNotBlank() } .forEach { album -> - albums += LibraryAlbum(boxId, artist, album, cachedAt = cachedAt) + albums += LibraryAlbum( + boxId = boxId, + albumArtist = artist, + album = album, + cachedAt = cachedAt, + provider = provider, + contentUri = contentUri, + contentType = contentType, + ) } } @@ -101,13 +130,41 @@ object LibraryParser { } } - return albums.distinctBy { it.albumArtist to it.album } + // Keyed on the content URI as well, so a record owned on CD *and* saved on a + // streaming service stays two rows. Collapsing them would hide whichever the box + // listed second, and the box lists its default backend first. + return albums.distinctBy { Triple(it.albumArtist, it.album, it.contentUri) } } + /** + * `list_library_sources` returns one `{id, label, views}` per registered backend. + * + * `views` is not read: it describes how each backend can be browsed, and Coil's own two + * tabs already match what the box does — `get_folder_content` is routed to the default + * backend whatever else is registered, so the folders tab is local-only by construction. + * Parsing a field nothing consumes would only be a second thing to keep true. + * + * An entry with no `id` is dropped rather than defaulted: the id is what a play call is + * routed by, and inventing one would send content to the wrong backend. + */ + fun librarySources(result: JsonElement?): List = + (result as? JsonArray).orEmpty().mapNotNull { element -> + val entry = element as? JsonObject ?: return@mapNotNull null + val id = entry.string("id")?.takeIf { it.isNotBlank() } ?: return@mapNotNull null + LibrarySource(id = id, label = entry.string("label")?.takeIf { it.isNotBlank() } ?: id) + } + /** Outcome of a cover art request. */ sealed interface CoverArt { - /** A file name in the box's cover cache. */ - data class Available(val fileName: String) : CoverArt + /** + * Where the artwork is: a file name in the box's cover cache, or an absolute URL + * when the backend that owns the content serves its own artwork. + * + * Not "a file name" any more, which is why it is no longer called one — + * [Box.coverUrl] is what decides which of the two this is and whether it may be + * loaded at all. + */ + data class Available(val coverRef: String) : CoverArt /** * The box has queued the extraction on its own worker thread and has nothing to @@ -127,6 +184,12 @@ object LibraryParser { * * Two sentinel values matter, both from `coverart_cache_manager.py`: `CACHE_PENDING` * while extraction is queued, and an empty string for "no artwork". + * + * **A provider-neutral box can answer with an absolute URL instead**, when the backend + * that owns the content serves its own artwork — a Spotify track answers with + * `https://i.scdn.co/image/…`. Those are kept whole: taking the last path segment, which + * is right for the box's own `some/dir/hash.jpg`, would reduce such a URL to its hash and + * leave [Box.coverUrl] no way to tell it was ever external. */ fun coverArt(result: JsonElement?): CoverArt { val value = (result as? JsonPrimitive)?.content?.trim() @@ -135,6 +198,7 @@ object LibraryParser { return when { value == CACHE_PENDING -> CoverArt.Pending value.isBlank() || value == "null" || value == "None" -> CoverArt.Missing + value.isExternalCoverRef() -> CoverArt.Available(value) else -> CoverArt.Available(value.substringAfterLast('/')) } } diff --git a/core-transport/src/main/kotlin/app/coilforphoniebox/transport/PhonieboxCommand.kt b/core-transport/src/main/kotlin/app/coilforphoniebox/transport/PhonieboxCommand.kt index 84f83c7..a84d94e 100644 --- a/core-transport/src/main/kotlin/app/coilforphoniebox/transport/PhonieboxCommand.kt +++ b/core-transport/src/main/kotlin/app/coilforphoniebox/transport/PhonieboxCommand.kt @@ -1,7 +1,9 @@ package app.coilforphoniebox.transport +import app.coilforphoniebox.domain.model.LibraryProvider import app.coilforphoniebox.domain.model.PlayTarget import app.coilforphoniebox.domain.model.RepeatMode +import kotlinx.serialization.json.JsonArray import kotlinx.serialization.json.JsonElement import kotlinx.serialization.json.JsonObject import kotlinx.serialization.json.JsonPrimitive @@ -60,10 +62,14 @@ data class PhonieboxCommand( * reachable from this app, which limits the damage if the box's unauthenticated RPC * port is ever accidentally exposed (§16). * - * Argument names are taken from the plugin signatures in - * `src/jukebox/components/player/playermpd/__init__.py` and - * `src/jukebox/components/volume/__init__.py` on `future3/main`, not from the web - * UI's command table, which omits several of them. + * Argument names are taken from the plugin signatures rather than from the web UI's command + * table, which omits several of them: `src/jukebox/components/playermpd/__init__.py` and + * `src/jukebox/components/volume/__init__.py` on `future3/main`, and + * `src/jukebox/components/player/coordinator.py` on `future3/develop`, where the + * provider-neutral player replaced `playermpd` (which survives there only as a compatibility + * shim). The method names and their meanings are the same on both; what `develop` adds is the + * optional `provider` and `content_uri` routing arguments — see [albumKwargs] for why those are + * sent only when they say something. * * **`as_thread` is never sent.** The implementation plan suggests it for slow calls, but * `jukebox/plugs.py` starts a daemon thread and returns the `Thread` object rather than @@ -178,6 +184,43 @@ object Commands { retryable = true, ) + /** + * The backends this box browses with, each `{id, label, views}`. + * + * Doubles as the capability probe for the whole provider-neutral surface: a box that + * answers this has [listLibraryItems] and the `provider`/`content_uri` fields too, and one + * that rejects it has none of them. That makes it one question rather than a fallback per + * call — see `LibraryRepositoryImpl.ensureLibrarySources`, and [playAt] for the same + * rejected-call-as-probe trick. + * + * Cheap despite the library timeout: the box builds the answer from the backends already + * registered in memory rather than reading any catalogue. + */ + val listLibrarySources = player( + "list_library_sources", + timeoutMillis = PhonieboxCommand.LIBRARY_TIMEOUT_MILLIS, + retryable = true, + ) + + /** + * Everything browsable of the given kinds, across every backend. + * + * The provider-aware replacement for [listAlbums], which on such a box is the same call + * wearing a name that stopped being true once the answer could contain playlists. + * [contentTypes] keeps the box from assembling more than the albums tab is going to draw, + * which matters on the socket it shares with its card reader (§6). + * + * Only sent to a box known to have it — see [listLibrarySources]. + */ + fun listLibraryItems(contentTypes: List) = player( + "list_library_items", + kwargs = mapOf( + "content_types" to JsonArray(contentTypes.map { JsonPrimitive(it) }), + ), + timeoutMillis = PhonieboxCommand.LIBRARY_TIMEOUT_MILLIS, + retryable = true, + ) + /** * The box's current MPD queue, one entry per track. * @@ -202,15 +245,20 @@ object Commands { retryable = true, ) - fun albumCoverArt(albumArtist: String, album: String) = player( + fun albumCoverArt( + albumArtist: String, + album: String, + provider: String? = null, + contentUri: String? = null, + ) = player( "get_album_coverart", - kwargs = mapOf( - "albumartist" to JsonPrimitive(albumArtist), - "album" to JsonPrimitive(album), - ), + kwargs = albumKwargs(albumArtist, album, provider, contentUri), retryable = true, ) + fun albumCoverArt(album: PlayTarget.Album) = + albumCoverArt(album.albumArtist, album.album, album.provider, album.contentUri) + /** * Starts the box's MPD database scan. `update_wait` blocks until the scan is * finished and is avoided for that reason (§6.4). @@ -226,9 +274,11 @@ object Commands { is PlayTarget.Album -> player( "play_album", - kwargs = mapOf( - "albumartist" to JsonPrimitive(target.albumArtist), - "album" to JsonPrimitive(target.album), + kwargs = albumKwargs( + target.albumArtist, + target.album, + target.provider, + target.contentUri, ), retryable = true, ) @@ -240,6 +290,38 @@ object Commands { ) } + /** + * `albumartist` and `album`, plus the two routing arguments **only when they say + * something**. + * + * This omission is what keeps Coil working against a box that predates the + * provider-neutral player. There, `play_album` and `get_album_coverart` are strictly + * two-argument, so an unconditional `content_uri=` would come back as + * `TypeError: play_album() got an unexpected keyword argument` — the same rejection + * [playAt] relies on as a probe, except here it would be a plain regression. Such a box + * also never *returns* a content URI, so there is never one to send and the payload + * stays byte-for-byte what it always was. + * + * `provider` is dropped when it is MPD for the same reason: MPD is the default backend + * everywhere, so naming it adds a kwarg without changing where the call lands. + * + * Omitting rather than sending null is deliberate — the coordinator's own + * `args = (…, content_uri) if content_uri else (…)` treats absent and null alike, and an + * explicit null would still be an unexpected keyword to an older box. + */ + private fun albumKwargs( + albumArtist: String, + album: String, + provider: String?, + contentUri: String?, + ): Map = buildMap { + put("albumartist", JsonPrimitive(albumArtist)) + put("album", JsonPrimitive(album)) + contentUri?.takeIf { it.isNotBlank() }?.let { put("content_uri", JsonPrimitive(it)) } + provider?.takeIf { it.isNotBlank() && it != LibraryProvider.MPD } + ?.let { put("provider", JsonPrimitive(it)) } + } + // ---------------------------------------------------------------- volume // No `get_volume`: the level arrives on the `volume.level` topic four times a second, diff --git a/core-transport/src/test/kotlin/app/coilforphoniebox/transport/CommandsTest.kt b/core-transport/src/test/kotlin/app/coilforphoniebox/transport/CommandsTest.kt index dd2152a..b01dd59 100644 --- a/core-transport/src/test/kotlin/app/coilforphoniebox/transport/CommandsTest.kt +++ b/core-transport/src/test/kotlin/app/coilforphoniebox/transport/CommandsTest.kt @@ -147,6 +147,55 @@ class CommandsTest { assertEquals("Hexerei", command.kwargs["album"]?.jsonPrimitive?.content) } + /** + * **The old-box guarantee.** `future3/main`'s `play_album` and `get_album_coverart` take + * exactly two arguments, so a `content_uri=` such a box never asked for comes back as + * `TypeError: … unexpected keyword argument` and the album simply does not play. A box + * like that also never hands out a content URI, so there is never one to send — and the + * payload therefore has to stay exactly what it has always been. + */ + @Test + fun `a local album sends nothing a pre-provider box would reject`() { + val play = Commands.play(PlayTarget.Album("Bibi Blocksberg", "Hexerei")) + assertEquals(setOf("albumartist", "album"), play.kwargs.keys) + + val cover = Commands.albumCoverArt("Bibi Blocksberg", "Hexerei") + assertEquals(setOf("albumartist", "album"), cover.kwargs.keys) + } + + /** An explicit `mpd` is still the default backend, so it too stays off the wire. */ + @Test + fun `naming the default provider adds nothing to the payload`() { + val command = Commands.play( + PlayTarget.Album("Bibi Blocksberg", "Hexerei", provider = "mpd"), + ) + + assertEquals(setOf("albumartist", "album"), command.kwargs.keys) + } + + /** + * With a handle to send, both routing arguments travel — this is what stops the + * coordinator falling through to MPD, finding nothing, and clearing the queue. + */ + @Test + fun `an album from another backend carries its provider and handle`() { + val target = PlayTarget.Album( + albumArtist = "Bibi Blocksberg", + album = "Hexerei", + provider = "spotify", + contentUri = "spotify:album:123", + ) + + val play = Commands.play(target) + assertEquals("spotify:album:123", play.kwargs["content_uri"]?.jsonPrimitive?.content) + assertEquals("spotify", play.kwargs["provider"]?.jsonPrimitive?.content) + + val cover = Commands.albumCoverArt(target) + assertEquals("get_album_coverart", cover.method) + assertEquals("spotify:album:123", cover.kwargs["content_uri"]?.jsonPrimitive?.content) + assertEquals("spotify", cover.kwargs["provider"]?.jsonPrimitive?.content) + } + /** * The distinction that keeps a lost reply from skipping two tracks: absolute commands * may be retried, relative ones may not. diff --git a/core-transport/src/test/kotlin/app/coilforphoniebox/transport/LibraryParserTest.kt b/core-transport/src/test/kotlin/app/coilforphoniebox/transport/LibraryParserTest.kt index 84337e1..f4e9a05 100644 --- a/core-transport/src/test/kotlin/app/coilforphoniebox/transport/LibraryParserTest.kt +++ b/core-transport/src/test/kotlin/app/coilforphoniebox/transport/LibraryParserTest.kt @@ -89,6 +89,115 @@ class LibraryParserTest { assertEquals("", albums.first().albumArtist) } + /** + * A box predating the provider-neutral player sends none of the three new fields, and + * what it means by that is the only thing it has ever had: the local library. + */ + @Test + fun `an entry with no provider fields reads as a local album`() { + val albums = LibraryParser.albums( + "box-1", + parse("""[{"albumartist":"Bibi","album":"Hexerei"}]"""), + 0L, + ) + + assertEquals("mpd", albums.single().provider) + assertNull(albums.single().contentUri) + assertEquals("album", albums.single().contentType) + } + + /** + * Once a box has two backends it returns both catalogues in one list, and these three + * fields are all that separate them. Losing them is what sends a streaming album to MPD, + * which finds nothing and clears the queue on the way. + */ + @Test + fun `provider, handle and kind are kept`() { + val raw = """ + [ + {"albumartist":"Bibi","album":"Hexerei","provider":"mpd", + "content_uri":null,"content_type":"album"}, + {"albumartist":"Nico","album":"Chill Mix","provider":"spotify", + "content_uri":"spotify:playlist:42","content_type":"playlist"} + ] + """.trimIndent() + + val albums = LibraryParser.albums("box-1", parse(raw), 0L) + + val local = albums.single { it.provider == "mpd" } + assertNull("MPD addresses its own albums by artist and album", local.contentUri) + + val remote = albums.single { it.provider == "spotify" } + assertEquals("spotify:playlist:42", remote.contentUri) + assertEquals("playlist", remote.contentType) + assertEquals("Chill Mix", remote.album) + } + + /** + * Owning a record on disc *and* saving it in a streaming account is ordinary, and the two + * are played by different calls — so they have to stay two rows. Keying the de-duplication + * on artist and album alone would hide whichever the box listed second. + */ + @Test + fun `the same album from two backends stays two entries`() { + val raw = """ + [ + {"albumartist":"Bibi","album":"Hexerei","provider":"mpd"}, + {"albumartist":"Bibi","album":"Hexerei","provider":"spotify", + "content_uri":"spotify:album:9"} + ] + """.trimIndent() + + val albums = LibraryParser.albums("box-1", parse(raw), 0L) + + assertEquals(2, albums.size) + assertEquals(setOf(null, "spotify:album:9"), albums.map { it.contentUri }.toSet()) + } + + /** A genuine duplicate — same backend, same handle — is still collapsed. */ + @Test + fun `a repeated entry is still de-duplicated`() { + val raw = """ + [ + {"albumartist":"Bibi","album":"Hexerei"}, + {"albumartist":"Bibi","album":"Hexerei"} + ] + """.trimIndent() + + assertEquals(1, LibraryParser.albums("box-1", parse(raw), 0L).size) + } + + @Test + fun `library sources are read as id and label`() { + val raw = """ + [ + {"id":"mpd","label":"Local","views":[{"id":"albums","kind":"items"}]}, + {"id":"spotify","label":"Spotify","views":[]} + ] + """.trimIndent() + + val sources = LibraryParser.librarySources(parse(raw)) + + assertEquals(listOf("mpd", "spotify"), sources.map { it.id }) + assertEquals(listOf("Local", "Spotify"), sources.map { it.label }) + } + + /** + * The id is what a play call is routed by, so an entry without one is dropped rather than + * given a made-up id that would send content to the wrong backend. A missing *label* is + * only a naming problem, so the id stands in. + */ + @Test + fun `a source without an id is dropped, one without a label keeps its id`() { + val raw = """[{"label":"Nameless"},{"id":"other"}]""" + + val sources = LibraryParser.librarySources(parse(raw)) + + assertEquals(1, sources.size) + assertEquals("other", sources.single().id) + assertEquals("other", sources.single().label) + } + @Test fun `cover art returns a bare file name`() { assertEquals( @@ -119,4 +228,22 @@ class LibraryParserTest { assertEquals(LibraryParser.CoverArt.Missing, LibraryParser.coverArt(parse("null"))) assertEquals(LibraryParser.CoverArt.Missing, LibraryParser.coverArt(null)) } + + /** + * A provider-neutral box answers for a Spotify track with the artwork's own URL. Keeping + * only the last path segment — right for the box's own `cache/hash.jpg` — would reduce it + * to a hash, and `Box.coverUrl` would then have no way to tell it was ever external and + * would hang it off the box's address, where it 404s. + */ + @Test + fun `an absolute url survives whole`() { + assertEquals( + LibraryParser.CoverArt.Available("https://i.scdn.co/image/ab67616d0000b273"), + LibraryParser.coverArt(parse("\"https://i.scdn.co/image/ab67616d0000b273\"")), + ) + assertEquals( + LibraryParser.CoverArt.Available("http://cdn.example/art/1.jpg"), + LibraryParser.coverArt(parse("\"http://cdn.example/art/1.jpg\"")), + ) + } } diff --git a/docs/protocol-notes.md b/docs/protocol-notes.md index 5822bc5..03dfedb 100644 --- a/docs/protocol-notes.md +++ b/docs/protocol-notes.md @@ -1,6 +1,7 @@ # Phoniebox v3 — protocol reference -Distilled from the source of `MiczFlor/RPi-Jukebox-RFID`, branch `future3/main`. +Distilled from the source of `MiczFlor/RPi-Jukebox-RFID`, branch `future3/main`, with the +differences on `future3/develop` called out where they exist — see "More than one player backend". Reviewed August 2026. **Everything here should be verified against a running box** before architecture is built on it — see `tools/probe_phoniebox.py`. --- @@ -126,6 +127,41 @@ Full reference: `src/webapp/src/commands/index.js` in the Phoniebox repository. `get_folder_content`, `list_albums`, `list_songs_by_artist_and_album(albumartist, album)`, `get_single_coverart`, `get_album_coverart`, `play_folder(folder)`, `play_album(albumartist, album)`, `play_single(song_url)`, `playlistinfo`, `update`, `update_wait` +### More than one player backend — `future3/develop` only + +[#2694](https://github.com/MiczFlor/RPi-Jukebox-RFID/pull/2694) ("provider-neutral player and library +contracts", merged 2026-08-04) replaced `playermpd` with a `PlayerCoordinator` in +`src/jukebox/components/player/`, which fans the same method names out to any number of registered +backends. [#2699](https://github.com/MiczFlor/RPi-Jukebox-RFID/pull/2699) adds Spotify as a second +one; it is `enabled: false` by default, so most boxes still have exactly one. + +**`player.ctrl` remains the right address on both branches.** `mpd_plugin.initialize_mpd_player` +registers with `package = plugs.loaded_as(module)`, and `loaded_as` resolves the **config alias**, +not the module path — `jukebox.default.yaml` maps `player: playermpd` on `future3/main` and +`player: player.plugin` on `future3/develop`. Do not read the module rename as an address change. + +Where it does matter: + +- **`list_albums` concatenates every backend's catalogue** once more than one is registered, and + each entry gains `provider`, `content_uri`, `content_type` and `cover_url`. MPD sets all four + itself, so this is one contract rather than a Spotify special case. A pre-#2694 box sends none of + them, which reads as `mpd` / no URI / `album`. +- **`content_uri` is load-bearing.** `_content_backend_name(provider, content_uri)` routes on it and + falls back to the *default* backend — MPD — when it is absent. `PlayerMPD.play_album` is `clear()` + then `findadd` then `play()`, and `findadd` matching nothing is not an error, so an album sent + without its URI **empties the queue and plays silence, with no error reply.** +- **Send `content_uri` and `provider` only when you have them.** On `future3/main`, `play_album` and + `get_album_coverart` take exactly two arguments and answer an unexpected keyword with a + `TypeError`. Such a box never returns a URI either, so there is never one to send. +- **`get_single_coverart` can answer with an absolute URL** instead of a name in the box's cover + cache, when the backend serves its own artwork — Spotify returns `https://i.scdn.co/image/…`. +- `list_library_sources` returns `{id, label, views}` per backend, and + `list_library_items(provider, content_types)` is the provider-aware replacement for `list_albums` + (identical rows today; each backend's implementation delegates to the other). Neither exists + before #2694, which makes the former a one-question capability probe for everything above. +- `provider` is a **free-form string** each backend names itself with at `register_backend`, not a + closed set. Echo an unknown one back rather than trying to model it. + `playlistinfo` returns the **current MPD queue**, one object per track with `file`, `pos`, `id`, `title`, `artist`, `album`, `track` and `duration`. It is a real `@plugs.tag` method on both `future3/main` and `future3/develop`, but the web UI never calls it, so it is absent from diff --git a/fastlane/metadata/android/de-DE/changelogs/8.txt b/fastlane/metadata/android/de-DE/changelogs/8.txt new file mode 100644 index 0000000..c72e9f4 --- /dev/null +++ b/fastlane/metadata/android/de-DE/changelogs/8.txt @@ -0,0 +1,3 @@ +Eine kommende Phoniebox-Version kann neben der eigenen Musik auch von einem Streamingdienst abspielen. Coil ist darauf vorbereitet: Die Albenliste zeigt beides, kennzeichnet die Quelle jedes Eintrags und startet das Richtige, auch wenn dasselbe Album an beiden Stellen liegt. + +Cover von so einem Dienst lassen sich jetzt ebenfalls anzeigen, wenn du das einschaltest. Standardmäßig bleibt es aus, damit Coil nur mit deiner Box spricht. diff --git a/fastlane/metadata/android/en-US/changelogs/8.txt b/fastlane/metadata/android/en-US/changelogs/8.txt new file mode 100644 index 0000000..051f005 --- /dev/null +++ b/fastlane/metadata/android/en-US/changelogs/8.txt @@ -0,0 +1,3 @@ +A coming Phoniebox release lets your box play from a streaming service as well as its own music. Coil is ready for it: the album list shows both, marks which source each entry came from, and starts the right one even when the same record is in both places. + +Cover art from such a service can now be shown too, if you switch it on. It stays off by default, so Coil only talks to your box. diff --git a/fastlane/metadata/android/es-ES/changelogs/8.txt b/fastlane/metadata/android/es-ES/changelogs/8.txt new file mode 100644 index 0000000..1bc74f8 --- /dev/null +++ b/fastlane/metadata/android/es-ES/changelogs/8.txt @@ -0,0 +1,3 @@ +Una próxima versión de Phoniebox permitirá que tu caja reproduzca desde un servicio de streaming además de su propia música. Coil ya está preparado: la lista de álbumes muestra ambos, indica de qué fuente viene cada entrada e inicia el correcto aunque el mismo disco esté en los dos sitios. + +Las carátulas de ese servicio también pueden mostrarse, si lo activas. Está desactivado por defecto, así Coil solo habla con tu caja. diff --git a/fastlane/metadata/android/fr-FR/changelogs/8.txt b/fastlane/metadata/android/fr-FR/changelogs/8.txt new file mode 100644 index 0000000..9c3e2fe --- /dev/null +++ b/fastlane/metadata/android/fr-FR/changelogs/8.txt @@ -0,0 +1,3 @@ +Une prochaine version de Phoniebox permettra à votre boîte de lire depuis un service de streaming en plus de sa propre musique. Coil est prêt : la liste des albums affiche les deux, indique la source de chaque entrée et lance le bon album même si le même se trouve des deux côtés. + +Les pochettes d'un tel service peuvent aussi s'afficher, si vous l'activez. C'est désactivé par défaut, pour que Coil ne parle qu'à votre boîte. diff --git a/fastlane/metadata/android/nl-NL/changelogs/8.txt b/fastlane/metadata/android/nl-NL/changelogs/8.txt new file mode 100644 index 0000000..d994933 --- /dev/null +++ b/fastlane/metadata/android/nl-NL/changelogs/8.txt @@ -0,0 +1,3 @@ +Een komende Phoniebox-versie laat je box behalve de eigen muziek ook van een streamingdienst afspelen. Coil is er klaar voor: de albumlijst toont beide, geeft van elke vermelding de bron aan en start de juiste, ook als hetzelfde album op beide plekken staat. + +Hoezen van zo'n dienst kunnen nu ook getoond worden, als je dat inschakelt. Standaard staat het uit, zodat Coil alleen met je box praat. diff --git a/feature-shortcuts/src/main/kotlin/app/coilforphoniebox/shortcuts/PlayDeepLink.kt b/feature-shortcuts/src/main/kotlin/app/coilforphoniebox/shortcuts/PlayDeepLink.kt index 1217514..c5c2253 100644 --- a/feature-shortcuts/src/main/kotlin/app/coilforphoniebox/shortcuts/PlayDeepLink.kt +++ b/feature-shortcuts/src/main/kotlin/app/coilforphoniebox/shortcuts/PlayDeepLink.kt @@ -4,6 +4,7 @@ import android.content.Intent import android.net.Uri import app.coilforphoniebox.domain.model.Favorite import app.coilforphoniebox.domain.model.FavoriteType +import app.coilforphoniebox.domain.model.LibraryProvider import app.coilforphoniebox.domain.model.PlayTarget /** @@ -26,6 +27,18 @@ object PlayDeepLink { private const val PARAM_URL = "url" private const val PARAM_FAVORITE = "favorite" + /** + * Which backend owns an album, and that backend's handle for it. + * + * Written only when they say something, and read with the same defaults as everywhere + * else — so a shortcut already sitting on someone's home screen, which has neither + * parameter, still parses and still means the local library, which is what it was pinned + * for. Without these two a pinned Spotify album would arrive as a bare artist-and-album + * pair and be played by the wrong backend. + */ + private const val PARAM_PROVIDER = "provider" + private const val PARAM_CONTENT_URI = "contenturi" + private const val TYPE_FOLDER = "folder" private const val TYPE_ALBUM = "album" private const val TYPE_TRACK = "track" @@ -49,10 +62,18 @@ object PlayDeepLink { .appendQueryParameter(PARAM_TYPE, TYPE_FOLDER) .appendQueryParameter(PARAM_PATH, favorite.folder ?: return null) - FavoriteType.ALBUM -> builder - .appendQueryParameter(PARAM_TYPE, TYPE_ALBUM) - .appendQueryParameter(PARAM_ALBUM_ARTIST, favorite.albumArtist ?: return null) - .appendQueryParameter(PARAM_ALBUM, favorite.album ?: return null) + FavoriteType.ALBUM -> { + builder + .appendQueryParameter(PARAM_TYPE, TYPE_ALBUM) + .appendQueryParameter(PARAM_ALBUM_ARTIST, favorite.albumArtist ?: return null) + .appendQueryParameter(PARAM_ALBUM, favorite.album ?: return null) + favorite.contentUri?.takeIf { it.isNotBlank() }?.let { + builder.appendQueryParameter(PARAM_CONTENT_URI, it) + } + favorite.provider.takeIf { it.isNotBlank() && it != LibraryProvider.MPD }?.let { + builder.appendQueryParameter(PARAM_PROVIDER, it) + } + } FavoriteType.TRACK -> builder .appendQueryParameter(PARAM_TYPE, TYPE_TRACK) @@ -81,7 +102,19 @@ object PlayDeepLink { TYPE_ALBUM -> { val artist = uri.getQueryParameter(PARAM_ALBUM_ARTIST) val album = uri.getQueryParameter(PARAM_ALBUM) - if (artist != null && album != null) PlayTarget.Album(artist, album) else null + if (artist != null && album != null) { + PlayTarget.Album( + albumArtist = artist, + album = album, + provider = uri.getQueryParameter(PARAM_PROVIDER) + ?.takeIf { it.isNotBlank() } + ?: LibraryProvider.MPD, + contentUri = uri.getQueryParameter(PARAM_CONTENT_URI) + ?.takeIf { it.isNotBlank() }, + ) + } else { + null + } } TYPE_TRACK -> uri.getQueryParameter(PARAM_URL)?.let { PlayTarget.Track(it) }