diff --git a/android/app/build.gradle.kts b/android/app/build.gradle.kts index 4e72d441..6a283978 100644 --- a/android/app/build.gradle.kts +++ b/android/app/build.gradle.kts @@ -173,6 +173,7 @@ dependencies { implementation(libs.media3.exoplayer) implementation(libs.media3.session) implementation(libs.media3.datasource.okhttp) + implementation(libs.mediarouter) implementation(libs.coil.compose) implementation(libs.coil.network.okhttp) @@ -186,6 +187,12 @@ dependencies { testImplementation(libs.mockk) testImplementation(libs.kotlinx.coroutines.test) testImplementation(libs.okhttp.mockwebserver) + // kxml2 — provides an org.xmlpull.v1 impl on the JVM unit-test + // classpath. Android's stock XmlPullParserFactory resolves to the + // android.jar Stub on JVM tests; kxml2 is picked up via service- + // provider lookup and makes XmlPullParserFactory.newInstance() work + // unconditionally so DeviceDescriptionTest runs in CI. + testImplementation(libs.kxml2) // kotlin.test for assertEquals/assertNull/etc. — version managed by // the applied Kotlin plugin so no explicit version pin needed. testImplementation(kotlin("test")) diff --git a/android/app/src/main/AndroidManifest.xml b/android/app/src/main/AndroidManifest.xml index 1ec6dad8..15066c95 100644 --- a/android/app/src/main/AndroidManifest.xml +++ b/android/app/src/main/AndroidManifest.xml @@ -9,6 +9,8 @@ + + > = - likes.observeLikedTracks() - .map { tracks -> tracks.mapTo(mutableSetOf()) { it.id } } + likes.observeLikedTrackIds() .stateIn( scope = viewModelScope, started = SharingStarted.WhileSubscribed(SHARE_STOP_TIMEOUT_MS), diff --git a/android/app/src/main/java/com/fabledsword/minstrel/likes/data/LikesRepository.kt b/android/app/src/main/java/com/fabledsword/minstrel/likes/data/LikesRepository.kt index c876c7b6..f5e009f5 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/likes/data/LikesRepository.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/likes/data/LikesRepository.kt @@ -110,6 +110,20 @@ class LikesRepository @Inject constructor( fun observeIsLiked(entityType: String, entityId: String): Flow = likeDao.observeIsLiked(currentUserId(), entityType, entityId) + /** + * Reactive set of liked track ids. Use this when the caller only + * needs "is X in liked set", NOT when it needs to render the liked + * tracks themselves — [observeLikedTracks] does a `mapNotNull` join + * against `trackDao` and drops any liked id whose track isn't in + * local cache yet, so an "is liked" UI built on `observeLikedTracks` + * misses cross-device likes whose track row hasn't cached. + * + * Used by playlist/album detail screens to color a row's like + * button independent of whether the track is in the local library. + */ + fun observeLikedTrackIds(): Flow> = + likeDao.observeLikedIdsOfType(currentUserId(), ENTITY_TRACK).map { it.toSet() } + /** One-shot snapshot of the liked track-id set — for the offline pool filter. */ suspend fun likedTrackIds(): Set = likeDao.observeLikedIdsOfType(currentUserId(), ENTITY_TRACK).first().toSet() diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/LikeMediaCallback.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/LikeMediaCallback.kt new file mode 100644 index 00000000..9af56f44 --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/LikeMediaCallback.kt @@ -0,0 +1,78 @@ +package com.fabledsword.minstrel.player + +import android.os.Bundle +import androidx.media3.session.MediaSession +import androidx.media3.session.SessionCommand +import androidx.media3.session.SessionResult +import com.fabledsword.minstrel.likes.data.LikesRepository +import com.fabledsword.minstrel.likes.data.LikesRepository.Companion.ENTITY_TRACK +import com.google.common.util.concurrent.Futures +import com.google.common.util.concurrent.ListenableFuture +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.launch + +/** + * MediaSession callback that adds the like/heart custom command to + * every connecting controller (notification, lock screen, Pixel Watch, + * Android Auto) and routes taps through [LikesRepository.toggleLike] + * so they inherit the offline-resilient MutationQueue path. + * + * The [onConnect] grant is REQUIRED — without it the button is + * silently invisible on some surfaces (Media3 issue #2679). See the + * spec at docs/superpowers/specs/2026-06-02-android-media3-like-button-design.md. + * + * Suspend work in [onCustomCommand] is launched on [scope] (a + * service-lifetime scope) so the callback returns synchronously and + * the controller does not block on Room + REST. State updates flow + * back through [LikesRepository.observeIsLiked] so the icon refreshes + * via the like-state job in [MinstrelPlayerService]. + */ +class LikeMediaCallback( + private val likes: LikesRepository, + private val scope: CoroutineScope, +) : MediaSession.Callback { + + override fun onConnect( + session: MediaSession, + controller: MediaSession.ControllerInfo, + ): MediaSession.ConnectionResult { + val grants = MediaSession.ConnectionResult.DEFAULT_SESSION_COMMANDS + .buildUpon() + .add(SessionCommand(CMD_TOGGLE_LIKE, Bundle.EMPTY)) + .build() + return MediaSession.ConnectionResult.AcceptedResultBuilder(session) + .setAvailableSessionCommands(grants) + .build() + } + + override fun onCustomCommand( + session: MediaSession, + controller: MediaSession.ControllerInfo, + customCommand: SessionCommand, + args: Bundle, + ): ListenableFuture { + val code = if (customCommand.customAction == CMD_TOGGLE_LIKE) { + launchToggleForCurrent(session) + } else { + SessionResult.RESULT_ERROR_NOT_SUPPORTED + } + return Futures.immediateFuture(SessionResult(code)) + } + + private fun launchToggleForCurrent(session: MediaSession): Int { + val mediaId = session.player.currentMediaItem?.mediaId + ?: return SessionResult.RESULT_INFO_SKIPPED + scope.launch { + val current = likes.observeIsLiked(ENTITY_TRACK, mediaId).first() + likes.toggleLike(ENTITY_TRACK, mediaId, !current) + } + return SessionResult.RESULT_SUCCESS + } + + companion object { + /** Custom-action key for the like/heart button. Namespaced so + * future custom commands (output picker, etc.) don't collide. */ + const val CMD_TOGGLE_LIKE: String = "minstrel.toggle_like" + } +} diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/MinstrelPlayerService.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/MinstrelPlayerService.kt index bc9cbba0..ae4f3eb3 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/MinstrelPlayerService.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/MinstrelPlayerService.kt @@ -2,11 +2,29 @@ package com.fabledsword.minstrel.player import android.app.PendingIntent import android.content.Intent +import android.os.Bundle +import androidx.media3.common.MediaItem import androidx.media3.common.Player +import androidx.media3.session.CommandButton import androidx.media3.session.MediaSession import androidx.media3.session.MediaSessionService +import androidx.media3.session.SessionCommand import com.fabledsword.minstrel.MainActivity +import com.fabledsword.minstrel.likes.data.LikesRepository +import com.fabledsword.minstrel.likes.data.LikesRepository.Companion.ENTITY_TRACK +import com.google.common.collect.ImmutableList import dagger.hilt.android.AndroidEntryPoint +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancel +import kotlinx.coroutines.channels.awaitClose +import kotlinx.coroutines.flow.callbackFlow +import kotlinx.coroutines.flow.flatMapLatest +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.flow.onStart +import kotlinx.coroutines.launch import javax.inject.Inject /** @@ -20,29 +38,49 @@ import javax.inject.Inject * MediaButtonReceiver is registered automatically by the library, no * manual receiver class needed. * + * The session advertises a single custom CommandButton — the like/heart + * toggle. [LikeMediaCallback] grants the command in `onConnect` and + * routes taps through [LikesRepository.toggleLike] so they inherit the + * offline-resilient MutationQueue path. The icon mirrors server state: + * a service-scoped coroutine collects `currentMediaItem × isLiked` and + * rebuilds the preferences list on each emission so a cross-device + * like (web tap) flips the notification heart automatically. + * * Lifecycle: - * - onCreate: build the ExoPlayer + MediaSession once. + * - onCreate: build the ExoPlayer + MediaSession once, attach the + * callback, set initial preferences, launch the like-state job. * - onGetSession: return the live session to any binding controller * (system UI media card, Wear OS companion, MediaController3 clients). * - onTaskRemoved: if the user swipes the app away, keep playing when * audio is active (standard media-app behavior — music shouldn't * die because the app left recents); otherwise stop the service so * the lingering notification clears. - * - onDestroy: release the session + player. + * - onDestroy: cancel the service scope, then release the session + + * player. Cancellation comes first so the like-state job doesn't + * touch a released session. */ @AndroidEntryPoint class MinstrelPlayerService : MediaSessionService() { @Inject lateinit var playerFactory: PlayerFactory + @Inject lateinit var likesRepository: LikesRepository + + private val serviceScope = CoroutineScope(SupervisorJob() + Dispatchers.Main.immediate) + private var mediaSession: MediaSession? = null override fun onCreate() { super.onCreate() val player = playerFactory.build() - mediaSession = MediaSession.Builder(this, player) + val callback = LikeMediaCallback(likesRepository, serviceScope) + val session = MediaSession.Builder(this, player) .setSessionActivity(buildNowPlayingPendingIntent()) + .setCallback(callback) + .setMediaButtonPreferences(ImmutableList.of(buildLikeButton(isLiked = false))) .build() + mediaSession = session + serviceScope.launch { observeLikeState(session, player) } } /** @@ -72,6 +110,67 @@ class MinstrelPlayerService : MediaSessionService() { ) } + /** + * Build the like/heart CommandButton. Icon flips between + * ICON_HEART_FILLED and ICON_HEART_UNFILLED to mirror server-side + * like state. The session command is the same in both states so + * the callback's onCustomCommand handles them identically (it + * inverts the current observed state regardless of which icon was + * tapped). + */ + private fun buildLikeButton(isLiked: Boolean): CommandButton { + val icon = if (isLiked) { + CommandButton.ICON_HEART_FILLED + } else { + CommandButton.ICON_HEART_UNFILLED + } + return CommandButton.Builder(icon) + .setDisplayName(if (isLiked) "Unlike" else "Like") + .setSessionCommand(SessionCommand(LikeMediaCallback.CMD_TOGGLE_LIKE, Bundle.EMPTY)) + .build() + } + + /** + * Collect player.currentMediaItem changes (via a Player.Listener + * lifted into a Flow) and, for each non-null mediaId, observe its + * liked state. On every emission, rebuild the session's media + * button preferences with the icon flipped accordingly. + * flatMapLatest cancels the previous track's subscription so we + * never leak Flows across track transitions. + */ + @OptIn(ExperimentalCoroutinesApi::class) + private suspend fun observeLikeState(session: MediaSession, player: Player) { + currentMediaIdFlow(player) + .flatMapLatest { mediaId -> + if (mediaId == null) { + flowOf(false) + } else { + likesRepository.observeIsLiked(ENTITY_TRACK, mediaId) + } + } + .collect { isLiked -> + session.setMediaButtonPreferences( + ImmutableList.of(buildLikeButton(isLiked = isLiked)), + ) + } + } + + /** + * Lift Player.currentMediaItem changes into a Flow. Emits the + * current mediaId on subscription so the initial icon state is + * correct on the first frame the controller renders (no + * unfilled-then-flips flicker). + */ + private fun currentMediaIdFlow(player: Player) = callbackFlow { + val listener = object : Player.Listener { + override fun onMediaItemTransition(item: MediaItem?, reason: Int) { + trySend(item?.mediaId) + } + } + player.addListener(listener) + awaitClose { player.removeListener(listener) } + }.onStart { emit(player.currentMediaItem?.mediaId) } + override fun onGetSession(controllerInfo: MediaSession.ControllerInfo): MediaSession? = mediaSession @@ -85,6 +184,7 @@ class MinstrelPlayerService : MediaSessionService() { } override fun onDestroy() { + serviceScope.cancel() mediaSession?.run { player.release() release() diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt index 102bcba1..0604cd03 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt @@ -3,6 +3,8 @@ package com.fabledsword.minstrel.player import android.content.ComponentName import android.content.Context import android.os.Bundle +import android.os.Handler +import android.os.Looper import androidx.media3.common.MediaItem import androidx.media3.common.MediaMetadata import androidx.media3.common.Player @@ -187,9 +189,26 @@ class PlayerController @Inject constructor( val controller = mediaController ?: return queueRefs = tracks val items = tracks.map { it.toMediaItem(source) } - controller.setMediaItems(items, initialIndex, /* startPositionMs = */ 0L) - controller.prepare() - if (autoplay) controller.play() + // Drift #562 cold-boot resume calls this from a non-Main suspend + // context after awaitReady() unblocks (ResumeController launches + // on Dispatchers.Default by the time it reaches us). MediaController + // enforces application-thread access and throws + // IllegalStateException otherwise — post to its applicationLooper + // if we're already there, run directly to avoid the re-dispatch + // latency UI callers depend on. + runOnControllerThread(controller) { + controller.setMediaItems(items, initialIndex, /* startPositionMs = */ 0L) + controller.prepare() + if (autoplay) controller.play() + } + } + + private fun runOnControllerThread(controller: MediaController, block: () -> Unit) { + if (Looper.myLooper() == controller.applicationLooper) { + block() + } else { + Handler(controller.applicationLooper).post(block) + } } /** diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/output/DeviceChip.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/output/DeviceChip.kt new file mode 100644 index 00000000..ebe28ca4 --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/output/DeviceChip.kt @@ -0,0 +1,84 @@ +package com.fabledsword.minstrel.player.output + +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size +import androidx.compose.material3.Icon +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Surface +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.graphics.vector.ImageVector +import androidx.compose.ui.text.style.TextOverflow +import androidx.compose.ui.unit.dp +import com.composables.icons.lucide.Bluetooth +import com.composables.icons.lucide.Cast +import com.composables.icons.lucide.ChevronDown +import com.composables.icons.lucide.Headphones +import com.composables.icons.lucide.Lucide +import com.composables.icons.lucide.Smartphone +import com.composables.icons.lucide.Speaker + +/** + * Spotify-style chip showing the current output route. Sits between + * BottomActionsRow and ScrubberRow in NowPlayingScreen. Tap to open + * the picker sheet. + * + * Visibility rule per the spec: hidden when the route list has + * exactly one entry AND that entry is BuiltIn — no reason to surface + * a picker for "the only thing available." Visibility logic owned + * by the caller (NowPlayingScreen). + */ +@Composable +fun DeviceChip( + route: OutputRoute, + onClick: () -> Unit, + modifier: Modifier = Modifier, +) { + Surface( + modifier = modifier, + onClick = onClick, + shape = MaterialTheme.shapes.medium, + color = MaterialTheme.colorScheme.surfaceVariant, + ) { + Row( + modifier = Modifier.padding(horizontal = 12.dp, vertical = 8.dp), + verticalAlignment = Alignment.CenterVertically, + horizontalArrangement = Arrangement.spacedBy(8.dp), + ) { + Icon( + imageVector = iconFor(route.kind), + contentDescription = null, + tint = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.size(CHIP_ICON_DP.dp), + ) + Text( + text = route.name, + style = MaterialTheme.typography.labelLarge, + color = MaterialTheme.colorScheme.onSurfaceVariant, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + ) + Icon( + imageVector = Lucide.ChevronDown, + contentDescription = null, + tint = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.size(CHIP_CHEVRON_DP.dp), + ) + } + } +} + +internal fun iconFor(kind: OutputRoute.Kind): ImageVector = when (kind) { + OutputRoute.Kind.BuiltIn -> Lucide.Smartphone + OutputRoute.Kind.Wired -> Lucide.Headphones + OutputRoute.Kind.Bluetooth -> Lucide.Bluetooth + OutputRoute.Kind.Cast -> Lucide.Cast + OutputRoute.Kind.Other -> Lucide.Speaker +} + +private const val CHIP_ICON_DP = 16 +private const val CHIP_CHEVRON_DP = 14 diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputPickerController.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputPickerController.kt new file mode 100644 index 00000000..76ee9b9b --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputPickerController.kt @@ -0,0 +1,222 @@ +package com.fabledsword.minstrel.player.output + +import android.content.Context +import androidx.mediarouter.media.MediaControlIntent +import androidx.mediarouter.media.MediaRouteSelector +import androidx.mediarouter.media.MediaRouter +import com.fabledsword.minstrel.api.endpoints.CastApi +import com.fabledsword.minstrel.api.endpoints.StreamTokenRequest +import com.fabledsword.minstrel.di.ApplicationScope +import com.fabledsword.minstrel.player.PlayerController +import com.fabledsword.minstrel.player.output.upnp.UpnpDiscoveryController +import dagger.hilt.android.qualifiers.ApplicationContext +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.SharingStarted +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.stateIn +import kotlinx.coroutines.launch +import retrofit2.Retrofit +import retrofit2.create +import timber.log.Timber +import javax.inject.Inject +import javax.inject.Singleton + +/** + * Snapshot of the audio output route state. [current] is the live + * route audio is being delivered to. [available] is every route the + * picker knows about — MediaRouter system routes merged with + * UPnP/DLNA renderers discovered on the LAN — sorted current-first + * then by [OutputRoute.Kind] (Bluetooth, Wired, BuiltIn, Other). + */ +data class RouteSnapshot( + val current: OutputRoute, + val available: List, +) + +/** + * Hilt-singleton facade over [MediaRouter] + [UpnpDiscoveryController]. + * Owns the callback lifecycle — default passive behavior at process + * start (route list updates without forcing Bluetooth scans), upgrades + * to active discovery while the picker sheet is open so newly-paired + * devices appear promptly. The [routesState] StateFlow projects the + * current route + sorted available list as a [RouteSnapshot]; the + * picker ViewModel collects it. + * + * Selection branches on [OutputRoute.Protocol]: + * - [OutputRoute.Protocol.SYSTEM] — MediaRouter.selectRoute (built-in, + * wired, Bluetooth) + * - [OutputRoute.Protocol.UPNP] — mint a signed stream token via + * [CastApi.streamToken], drive the discovered renderer with + * AVTransport.SetAVTransportURI + Play, pause local playback so + * audio yields to the network speaker + * - [OutputRoute.Protocol.CAST] / [OutputRoute.Protocol.SONOS] — + * reserved for follow-up slices; ignored for now. + * + * Mirrors the OutputPickerController role described in + * docs/superpowers/specs/2026-06-03-android-output-picker-bluetooth-design.md + * and the UPnP extensions in + * docs/superpowers/specs/2026-06-03-android-output-picker-upnp-design.md. + */ +@Singleton +class OutputPickerController @Inject constructor( + @ApplicationContext private val context: Context, + @ApplicationScope private val scope: CoroutineScope, + private val upnpDiscovery: UpnpDiscoveryController, + private val playerController: PlayerController, + retrofit: Retrofit, +) { + private val castApi: CastApi = retrofit.create() + + private val mediaRouter = MediaRouter.getInstance(context) + + private val selector = MediaRouteSelector.Builder() + .addControlCategory(MediaControlIntent.CATEGORY_LIVE_AUDIO) + .build() + + /** + * Internal projection of the live MediaRouter snapshot. Refreshed + * on every Callback event; combined downstream with UPnP routes + * into the public [routesState]. + */ + private val systemRoutesInternal = MutableStateFlow(snapshotFromRouter()) + + val routesState: StateFlow = combine( + systemRoutesInternal, + upnpDiscovery.routes, + ) { sys, upnp -> + val merged = sys.available + upnp.map { OutputRoute.fromUpnpRoute(it) } + RouteSnapshot(current = sys.current, available = sortRoutes(sys.current, merged)) + }.stateIn(scope, SharingStarted.Eagerly, systemRoutesInternal.value) + + private val callback = object : MediaRouter.Callback() { + override fun onRouteAdded(router: MediaRouter, route: MediaRouter.RouteInfo) = + refresh() + + override fun onRouteChanged(router: MediaRouter, route: MediaRouter.RouteInfo) = + refresh() + + override fun onRouteRemoved(router: MediaRouter, route: MediaRouter.RouteInfo) = + refresh() + + override fun onRouteSelected( + router: MediaRouter, + route: MediaRouter.RouteInfo, + reason: Int, + ) = refresh() + + override fun onRouteUnselected( + router: MediaRouter, + route: MediaRouter.RouteInfo, + reason: Int, + ) = refresh() + } + + init { + // Two-arg addCallback registers with no discovery flag — + // androidx.mediarouter 1.7.0's default passive behavior: + // route-list updates flow through onRouteAdded/Removed/Changed + // without forcing Bluetooth scans. (There is no + // CALLBACK_FLAG_PASSIVE_DISCOVERY constant; absent flag = passive.) + mediaRouter.addCallback(selector, callback) + } + + /** + * Upgrade callback registration to active discovery — call when + * the picker sheet opens so newly-paired Bluetooth devices + * surface within a few seconds, and fire an SSDP M-SEARCH burst + * for UPnP renderers. Idempotent: re-registering with a + * new flag set replaces the prior registration in MediaRouter. + */ + fun upgradeDiscovery() { + mediaRouter.addCallback(selector, callback, MediaRouter.CALLBACK_FLAG_REQUEST_DISCOVERY) + upnpDiscovery.upgradeDiscovery() + } + + /** + * Downgrade callback registration back to default passive behavior + * — call when the picker sheet closes so we don't keep Bluetooth + * scanning on for battery cost. Two-arg overload = no flag = + * passive. Same idempotent re-registration semantics. The UPnP + * side has no symmetric downgrade (passive SSDP NOTIFY listen is + * always on); the call is preserved for API parity. + */ + fun downgradeDiscovery() { + mediaRouter.addCallback(selector, callback) + upnpDiscovery.downgradeDiscovery() + } + + /** + * Select [route]. SYSTEM routes hand off to [MediaRouter]; UPNP + * routes mint a signed stream token + drive AVTransport on the + * discovered renderer + pause local playback. Other protocols + * (CAST / SONOS) are reserved for follow-up slices and are + * silently ignored — the picker shouldn't show them yet. + */ + fun select(route: OutputRoute) { + when (route.protocol) { + OutputRoute.Protocol.SYSTEM -> selectSystem(route) + OutputRoute.Protocol.UPNP -> scope.launch { selectUpnp(route) } + OutputRoute.Protocol.CAST, OutputRoute.Protocol.SONOS -> Unit + } + } + + private fun selectSystem(route: OutputRoute) { + val target = mediaRouter.routes.firstOrNull { it.id == route.id } ?: return + mediaRouter.selectRoute(target) + } + + /** + * Drive the UPnP renderer: mint a token for the currently playing + * track, set the renderer's URI, play, then pause local playback so + * audio yields to the speaker. Wrapped in `runCatching` at each + * step — token failure, transport-lookup failure, and SOAP failure + * each abandon the selection cleanly rather than crashing. Failures + * log at warn level via Timber so on-device verification can find + * the cause in logcat (OkHttp's logger doesn't cover our own + * deserialize / SOAP-parse code paths). + */ + private suspend fun selectUpnp(route: OutputRoute) { + val trackId = playerController.uiState.value.currentTrack?.id ?: return + val transport = upnpDiscovery.transportFor(route.id) ?: return + runCatching { + val token = castApi.streamToken(StreamTokenRequest(trackId = trackId)) + transport.setAVTransportURI(token.url) + transport.play() + playerController.pause() + }.onFailure { e -> + Timber.w(e, "UPnP select failed for route ${route.id}") + } + } + + private fun refresh() { + systemRoutesInternal.value = snapshotFromRouter() + } + + private fun snapshotFromRouter(): RouteSnapshot { + val all = mediaRouter.routes + .filter { it.matchesSelector(selector) } + .map { OutputRoute.fromRouteInfo(it) } + val current = OutputRoute.fromRouteInfo(mediaRouter.selectedRoute) + return RouteSnapshot(current = current, available = sortRoutes(current, all)) + } + + /** + * Selected first, then Bluetooth, then Wired, then BuiltIn, then + * Other (UPnP renderers fall in Other). Keeps the active output at + * the top + likely-wanted alternatives next + fallback last. + */ + private fun sortRoutes(current: OutputRoute, all: List): List { + val rank: (OutputRoute) -> Int = { route -> + when { + route.id == current.id -> 0 + route.kind == OutputRoute.Kind.Bluetooth -> 1 + route.kind == OutputRoute.Kind.Wired -> 2 + route.kind == OutputRoute.Kind.BuiltIn -> 3 + else -> 4 + } + } + return all.sortedBy(rank) + } +} diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputPickerSheet.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputPickerSheet.kt new file mode 100644 index 00000000..173fcb46 --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputPickerSheet.kt @@ -0,0 +1,200 @@ +package com.fabledsword.minstrel.player.output + +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size +import androidx.compose.material3.ExperimentalMaterial3Api +import androidx.compose.material3.Icon +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.ModalBottomSheet +import androidx.compose.material3.Surface +import androidx.compose.material3.Text +import androidx.compose.material3.rememberModalBottomSheetState +import androidx.compose.runtime.Composable +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.text.style.TextOverflow +import androidx.compose.ui.unit.dp +import com.composables.icons.lucide.Circle +import com.composables.icons.lucide.CircleCheck +import com.composables.icons.lucide.Lucide +import com.composables.icons.lucide.Settings +import com.composables.icons.lucide.WifiOff + +/** + * Material 3 ModalBottomSheet listing the available output routes. + * Tap a row to select + dismiss. Selected route shows the accent + * CircleCheck; others show an empty Circle. Long route names + * truncate cleanly via maxLines = 2 + Ellipsis. + * + * Two footer hints, each driven by a flag the host screen owns: + * - [permissionDenied] — NowPlayingScreen owns the BLUETOOTH_CONNECT + * request flow and sets this true on denial so the user sees the + * "pair in Settings" affordance. + * - [noUpnpDiscovered] — set true when the picker has been open for + * a few seconds and no UPnP renderers have arrived; surfaces the + * "router may be blocking multicast" hint. Defaults to false so + * existing call sites that haven't wired the discovery-timing + * logic continue to render without the hint. + * + * Keeping the hint flags external preserves this composable's + * focus-on-rendering shape. + */ +@OptIn(ExperimentalMaterial3Api::class) +@Composable +fun OutputPickerSheet( + snapshot: RouteSnapshot, + permissionDenied: Boolean, + onRouteSelected: (OutputRoute) -> Unit, + onDismiss: () -> Unit, + noUpnpDiscovered: Boolean = false, +) { + val sheetState = rememberModalBottomSheetState() + ModalBottomSheet( + onDismissRequest = onDismiss, + sheetState = sheetState, + ) { + Column( + modifier = Modifier + .fillMaxWidth() + .padding(horizontal = 24.dp, vertical = 8.dp), + verticalArrangement = Arrangement.spacedBy(4.dp), + ) { + Text( + text = "Output", + style = MaterialTheme.typography.titleMedium, + color = MaterialTheme.colorScheme.onSurface, + modifier = Modifier.padding(vertical = 8.dp), + ) + snapshot.available.forEach { route -> + RouteRow( + route = route, + isSelected = route.id == snapshot.current.id, + onClick = { onRouteSelected(route) }, + ) + } + if (permissionDenied) { + PermissionHintRow() + } + if (noUpnpDiscovered) { + MulticastHintRow() + } + } + } +} + +@Composable +private fun RouteRow( + route: OutputRoute, + isSelected: Boolean, + onClick: () -> Unit, +) { + val tint = if (isSelected) { + MaterialTheme.colorScheme.primary + } else { + MaterialTheme.colorScheme.onSurfaceVariant + } + Surface( + onClick = onClick, + color = MaterialTheme.colorScheme.surface, + modifier = Modifier.fillMaxWidth(), + ) { + Row( + modifier = Modifier + .fillMaxWidth() + .padding(vertical = 12.dp), + verticalAlignment = Alignment.CenterVertically, + horizontalArrangement = Arrangement.spacedBy(16.dp), + ) { + Icon( + imageVector = iconFor(route.kind), + contentDescription = null, + tint = tint, + modifier = Modifier.size(ROW_ICON_DP.dp), + ) + Column(modifier = Modifier.weight(1f)) { + Text( + text = route.name, + style = MaterialTheme.typography.bodyLarge, + color = MaterialTheme.colorScheme.onSurface, + maxLines = 2, + overflow = TextOverflow.Ellipsis, + ) + val subtitle = route.description ?: defaultSubtitle(route) + Text( + text = subtitle, + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + ) + } + Icon( + imageVector = if (isSelected) Lucide.CircleCheck else Lucide.Circle, + contentDescription = if (isSelected) "Selected" else "Not selected", + tint = tint, + modifier = Modifier.size(ROW_ICON_DP.dp), + ) + } + } +} + +@Composable +private fun PermissionHintRow() { + Row( + modifier = Modifier + .fillMaxWidth() + .padding(vertical = 12.dp), + verticalAlignment = Alignment.CenterVertically, + horizontalArrangement = Arrangement.spacedBy(12.dp), + ) { + Icon( + imageVector = Lucide.Settings, + contentDescription = null, + tint = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.size(HINT_ICON_DP.dp), + ) + Text( + text = "Pair a Bluetooth device in Settings to see it here.", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } +} + +@Composable +private fun MulticastHintRow() { + Row( + modifier = Modifier + .fillMaxWidth() + .padding(vertical = 12.dp), + verticalAlignment = Alignment.CenterVertically, + horizontalArrangement = Arrangement.spacedBy(12.dp), + ) { + Icon( + imageVector = Lucide.WifiOff, + contentDescription = null, + tint = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.size(HINT_ICON_DP.dp), + ) + Text( + text = "Your router may be blocking multicast discovery.", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } +} + +private fun defaultSubtitle(route: OutputRoute): String = when (route.kind) { + OutputRoute.Kind.BuiltIn -> "Phone speaker" + OutputRoute.Kind.Wired -> "Wired" + OutputRoute.Kind.Bluetooth -> if (route.isConnected) "Connected" else "Available" + OutputRoute.Kind.Cast -> "Cast" + OutputRoute.Kind.Other -> "Available" +} + +private const val ROW_ICON_DP = 24 +private const val HINT_ICON_DP = 20 diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputPickerViewModel.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputPickerViewModel.kt new file mode 100644 index 00000000..19362d60 --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputPickerViewModel.kt @@ -0,0 +1,60 @@ +package com.fabledsword.minstrel.player.output + +import androidx.lifecycle.ViewModel +import androidx.lifecycle.viewModelScope +import dagger.hilt.android.lifecycle.HiltViewModel +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.SharingStarted +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.stateIn +import javax.inject.Inject + +/** + * NowPlaying-scoped projection over [OutputPickerController]. The + * controller is the long-lived Hilt singleton (its MediaRouter + * callback must not churn with screen lifecycle); this ViewModel is + * a thin lens over its [OutputPickerController.routesState] Flow + * plus the sheet's visibility state. + * + * Sheet open/close are forwarded to the controller's discovery + * toggle so active MediaRouter discovery only runs while the sheet + * is actually visible. + */ +@HiltViewModel +class OutputPickerViewModel @Inject constructor( + private val controller: OutputPickerController, +) : ViewModel() { + + val routes: StateFlow = controller.routesState + .stateIn( + scope = viewModelScope, + started = SharingStarted.WhileSubscribed(STOP_TIMEOUT_MS), + initialValue = controller.routesState.value, + ) + + private val sheetVisibleInternal = MutableStateFlow(false) + val sheetVisible: StateFlow = sheetVisibleInternal.asStateFlow() + + fun onChipTapped() { + sheetVisibleInternal.value = true + controller.upgradeDiscovery() + } + + fun onSheetDismissed() { + sheetVisibleInternal.value = false + controller.downgradeDiscovery() + } + + fun onRouteSelected(route: OutputRoute) { + controller.select(route) + sheetVisibleInternal.value = false + controller.downgradeDiscovery() + } + + private companion object { + // SharingStarted timeout so quick screen-orientation changes + // don't tear down + re-subscribe the controller's Flow. + const val STOP_TIMEOUT_MS = 5_000L + } +} diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputRoute.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputRoute.kt new file mode 100644 index 00000000..7b7944f2 --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputRoute.kt @@ -0,0 +1,103 @@ +package com.fabledsword.minstrel.player.output + +import androidx.mediarouter.media.MediaRouter +import com.fabledsword.minstrel.player.output.upnp.UpnpRoute + +/** + * Narrow domain model for an audio output route. Independent of + * [MediaRouter.RouteInfo] so the picker UI can preview without an + * Android framework presence (RouteInfo can't be constructed in + * JVM tests, mirroring the LikeMediaCallback constraint). + * + * The [protocol] field is a forward-compatibility hook for the + * UPnP / DLNA / Sonos / Cast slice (scope captured in + * docs/superpowers/specs/2026-06-03-android-output-picker-upnp-scope.md). + * THIS slice always sets `protocol = SYSTEM` — the system-route + * categories MediaRouter surfaces (built-in / wired / Bluetooth). + */ +data class OutputRoute( + val id: String, + val name: String, + val description: String?, + val kind: Kind, + val protocol: Protocol, + val isConnected: Boolean, +) { + enum class Kind { BuiltIn, Wired, Bluetooth, Cast, Other } + + enum class Protocol { + /** System-managed routes — built-in speaker, wired, Bluetooth. */ + SYSTEM, + + /** Generic UPnP / DLNA renderers — reserved for the next slice. */ + UPNP, + + /** Chromecast via the Cast SDK — reserved for a later slice. */ + CAST, + + /** Sonos via the Sonos extension on top of UPnP — reserved. */ + SONOS, + } + + companion object { + /** + * Lift a MediaRouter [route] into the domain model. Kind is + * inferred from [MediaRouter.RouteInfo.getDeviceType]; unknown + * device types fall through to [Kind.Other]. The + * `connectionState` proxy is good enough for the chip's + * "Connected"/"Available" subtitle. + */ + fun fromRouteInfo(route: MediaRouter.RouteInfo): OutputRoute { + val kind = when (route.deviceType) { + MediaRouter.RouteInfo.DEVICE_TYPE_BUILTIN_SPEAKER -> Kind.BuiltIn + MediaRouter.RouteInfo.DEVICE_TYPE_WIRED_HEADSET, + MediaRouter.RouteInfo.DEVICE_TYPE_WIRED_HEADPHONES, + -> Kind.Wired + MediaRouter.RouteInfo.DEVICE_TYPE_BLUETOOTH_A2DP -> Kind.Bluetooth + MediaRouter.RouteInfo.DEVICE_TYPE_TV -> Kind.Other + MediaRouter.RouteInfo.DEVICE_TYPE_SPEAKER -> Kind.Other + else -> Kind.Other + } + val connected = + route.connectionState == MediaRouter.RouteInfo.CONNECTION_STATE_CONNECTED + return OutputRoute( + id = route.id, + name = route.name, + description = route.description, + kind = kind, + protocol = Protocol.SYSTEM, + isConnected = connected, + ) + } + + /** + * Lift a discovered UPnP renderer into the picker's domain + * model. Used by the UPnP discovery controller to merge + * network speakers into the same `OutputPickerController` + * routes stream the system routes come through. + * + * `isConnected = false` because UPnP devices have no + * MediaRouter connection-state concept — they're always + * "available" on the LAN, and the picker's selected-route + * rendering handles the "currently playing" indicator. + * + * Subtitle is `manufacturer modelName` joined by a single + * space, falling back to "Network speaker" when both fields + * are blank. + */ + fun fromUpnpRoute(route: UpnpRoute): OutputRoute { + val description = listOfNotNull( + route.manufacturer.takeIf { it.isNotBlank() }, + route.modelName.takeIf { it.isNotBlank() }, + ).joinToString(" ").ifBlank { "Network speaker" } + return OutputRoute( + id = route.id, + name = route.name, + description = description, + kind = Kind.Other, + protocol = Protocol.UPNP, + isConnected = false, + ) + } + } +} diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/AVTransportClient.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/AVTransportClient.kt new file mode 100644 index 00000000..a7897087 --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/AVTransportClient.kt @@ -0,0 +1,50 @@ +package com.fabledsword.minstrel.player.output.upnp + +import okhttp3.HttpUrl + +/** + * High-level wrapper for the UPnP AVTransport service. Three calls + * for v1: SetAVTransportURI / Play / Stop. Pause + Seek deferred + * until we have hardware in the loop to verify each device's quirks + * (Sonos and BubbleUPnP accept the standard shape; some smart TVs + * reject Pause without DIDL). + */ +class AVTransportClient( + private val soap: SoapClient, + private val controlUrl: HttpUrl, +) { + suspend fun setAVTransportURI(uri: String, metadata: String = "") { + soap.call( + controlUrl = controlUrl, + serviceType = SERVICE_TYPE, + action = "SetAVTransportURI", + args = mapOf( + "InstanceID" to "0", + "CurrentURI" to uri, + "CurrentURIMetaData" to metadata, + ), + ) + } + + suspend fun play() { + soap.call( + controlUrl = controlUrl, + serviceType = SERVICE_TYPE, + action = "Play", + args = mapOf("InstanceID" to "0", "Speed" to "1"), + ) + } + + suspend fun stop() { + soap.call( + controlUrl = controlUrl, + serviceType = SERVICE_TYPE, + action = "Stop", + args = mapOf("InstanceID" to "0"), + ) + } + + private companion object { + const val SERVICE_TYPE = "urn:schemas-upnp-org:service:AVTransport:1" + } +} diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/DeviceDescription.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/DeviceDescription.kt new file mode 100644 index 00000000..e672deb6 --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/DeviceDescription.kt @@ -0,0 +1,122 @@ +package com.fabledsword.minstrel.player.output.upnp + +import okhttp3.HttpUrl +import okhttp3.HttpUrl.Companion.toHttpUrlOrNull +import org.xmlpull.v1.XmlPullParser +import org.xmlpull.v1.XmlPullParserFactory + +/** + * Pull-parsed UPnP device description (the XML returned from a + * discovered LOCATION URL). Carries the bits we actually need for + * the picker: friendlyName / manufacturer / modelName for display, + * AVTransport + RenderingControl control URLs for command dispatch. + * + * Filtered to MediaRenderer-capable devices — anything without an + * AVTransport service control URL is dropped by [parse] returning + * null (we can't make it play). + */ +data class DeviceDescription( + val udn: String, + val friendlyName: String, + val manufacturer: String, + val modelName: String, + val avTransportControlUrl: HttpUrl, + val renderingControlUrl: HttpUrl?, +) { + companion object { + private const val AVT_SERVICE_TYPE = "urn:schemas-upnp-org:service:AVTransport:1" + private const val RC_SERVICE_TYPE = "urn:schemas-upnp-org:service:RenderingControl:1" + + private const val TAG_SERVICE = "service" + private const val TAG_UDN = "UDN" + private const val TAG_FRIENDLY_NAME = "friendlyName" + private const val TAG_MANUFACTURER = "manufacturer" + private const val TAG_MODEL_NAME = "modelName" + private const val TAG_SERVICE_TYPE = "serviceType" + private const val TAG_CONTROL_URL = "controlURL" + + /** + * Parse [xml] (the body fetched from the SSDP LOCATION URL), + * resolving relative service control URLs against [base]. + * Returns null when AVTransport is missing — we have no way + * to control the device without it. + */ + fun parse(xml: String, base: HttpUrl): DeviceDescription? { + val parser = XmlPullParserFactory.newInstance().newPullParser().apply { + setInput(xml.reader()) + } + val acc = ParseState() + while (parser.eventType != XmlPullParser.END_DOCUMENT) { + handleEvent(parser, acc, base) + parser.next() + } + val avt = acc.avtControlUrl ?: return null + return DeviceDescription( + udn = acc.udn, + friendlyName = acc.friendlyName, + manufacturer = acc.manufacturer, + modelName = acc.modelName, + avTransportControlUrl = avt, + renderingControlUrl = acc.rcControlUrl, + ) + } + + private fun handleEvent(parser: XmlPullParser, acc: ParseState, base: HttpUrl) { + when (parser.eventType) { + XmlPullParser.START_TAG -> handleStartTag(parser, acc) + XmlPullParser.END_TAG -> handleEndTag(parser, acc, base) + else -> Unit + } + } + + private fun handleStartTag(parser: XmlPullParser, acc: ParseState) { + when (parser.name) { + TAG_SERVICE -> { + acc.inService = true + acc.serviceType = "" + acc.serviceControlUrl = "" + } + TAG_UDN -> acc.udn = parser.nextTextSafe() + TAG_FRIENDLY_NAME -> acc.friendlyName = parser.nextTextSafe() + TAG_MANUFACTURER -> acc.manufacturer = parser.nextTextSafe() + TAG_MODEL_NAME -> acc.modelName = parser.nextTextSafe() + TAG_SERVICE_TYPE -> if (acc.inService) acc.serviceType = parser.nextTextSafe() + TAG_CONTROL_URL -> if (acc.inService) acc.serviceControlUrl = parser.nextTextSafe() + } + } + + private fun handleEndTag(parser: XmlPullParser, acc: ParseState, base: HttpUrl) { + if (parser.name != TAG_SERVICE) return + val resolved = resolveControlUrl(base, acc.serviceControlUrl) + when (acc.serviceType) { + AVT_SERVICE_TYPE -> acc.avtControlUrl = resolved + RC_SERVICE_TYPE -> acc.rcControlUrl = resolved + } + acc.inService = false + } + + private fun resolveControlUrl(base: HttpUrl, path: String): HttpUrl? { + if (path.isBlank()) return null + return path.toHttpUrlOrNull() ?: base.resolve(path) + } + + private fun XmlPullParser.nextTextSafe(): String = + runCatching { nextText() }.getOrDefault("") + } + + /** + * Mutable accumulator used during pull-parsing. Lives only for the + * duration of one [parse] call. + */ + private class ParseState { + var udn: String = "" + var friendlyName: String = "" + var manufacturer: String = "" + var modelName: String = "" + var avtControlUrl: HttpUrl? = null + var rcControlUrl: HttpUrl? = null + var inService: Boolean = false + var serviceType: String = "" + var serviceControlUrl: String = "" + } +} diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/SoapClient.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/SoapClient.kt new file mode 100644 index 00000000..4bf6ab51 --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/SoapClient.kt @@ -0,0 +1,140 @@ +package com.fabledsword.minstrel.player.output.upnp + +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import okhttp3.HttpUrl +import okhttp3.OkHttpClient +import okhttp3.Request +import okhttp3.RequestBody.Companion.toRequestBody +import org.xmlpull.v1.XmlPullParser +import org.xmlpull.v1.XmlPullParserFactory + +/** + * Minimal SOAP/UPnP envelope builder + POST. Hand-rolled rather than + * pulled in via jupnp; we control every line and integrate cleanly + * with the app's OkHttpClient (shared connection pool, timeouts). + * + * Builds the standard SOAP 1.1 envelope, POSTs with the required + * SOAPACTION + Content-Type headers, returns the parsed + * `Response` element as a Map (UPnP responses + * are flat string maps). + * + * Throws [SoapFaultException] on a `` response (the UPnP + * device's way of saying "I rejected your request"). Other transport + * errors propagate as IOException. + */ +class SoapClient( + private val okHttp: OkHttpClient, +) { + suspend fun call( + controlUrl: HttpUrl, + serviceType: String, + action: String, + args: Map = emptyMap(), + ): Map = withContext(Dispatchers.IO) { + val envelope = buildEnvelope(serviceType, action, args) + val request = Request.Builder() + .url(controlUrl) + .post(envelope.toRequestBody(null)) + .header("Content-Type", SOAP_CONTENT_TYPE) + .header("SOAPACTION", "\"$serviceType#$action\"") + .build() + okHttp.newCall(request).execute().use { response -> + val body = response.body?.string().orEmpty() + if (!response.isSuccessful) { + throw SoapFaultException(faultCodeOf(body), faultDescriptionOf(body)) + } + parseResponseArgs(body, action) + } + } + + private fun buildEnvelope( + serviceType: String, + action: String, + args: Map, + ): String = buildString { + append("") + append("") + append("") + append("") + args.forEach { (k, v) -> + append("<").append(k).append(">") + append(xmlEscape(v)) + append("") + } + append("") + append("") + append("") + } + + private fun xmlEscape(v: String): String = v + .replace("&", "&") + .replace("<", "<") + .replace(">", ">") + .replace("\"", """) + .replace("'", "'") + + private fun parseResponseArgs(body: String, action: String): Map { + val parser = XmlPullParserFactory.newInstance().newPullParser().apply { + setInput(body.reader()) + } + val responseTag = "${action}Response" + val args = mutableMapOf() + var inResponse = false + while (parser.eventType != XmlPullParser.END_DOCUMENT) { + inResponse = handleParserEvent(parser, responseTag, inResponse, args) + parser.next() + } + return args + } + + private fun handleParserEvent( + parser: XmlPullParser, + responseTag: String, + inResponse: Boolean, + args: MutableMap, + ): Boolean = when (parser.eventType) { + XmlPullParser.START_TAG -> { + if (parser.name == responseTag) { + true + } else { + if (inResponse) { + val name = parser.name + val text = runCatching { parser.nextText() }.getOrDefault("") + args[name] = text + } + inResponse + } + } + XmlPullParser.END_TAG -> if (parser.name == responseTag) false else inResponse + else -> inResponse + } + + private fun faultCodeOf(body: String): String = + extractBetween(body, "", "") ?: "unknown" + + private fun faultDescriptionOf(body: String): String = + extractBetween(body, "", "").orEmpty() + + private fun extractBetween(body: String, open: String, close: String): String? { + val start = body.indexOf(open) + if (start < 0) return null + val contentStart = start + open.length + val end = body.indexOf(close, contentStart) + return if (end < 0) null else body.substring(contentStart, end) + } + + private companion object { + const val SOAP_CONTENT_TYPE = "text/xml; charset=\"utf-8\"" + } +} + +/** + * Thrown when a UPnP device responds with `` — typically wraps + * a UPnPError with `errorCode` + `errorDescription`. Code is preserved + * as a string (UPnP codes are numeric in spec but we don't constrain). + */ +class SoapFaultException(val code: String, val description: String) : + Exception("SOAP fault $code: $description") diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/SsdpDiscovery.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/SsdpDiscovery.kt new file mode 100644 index 00000000..13984112 --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/SsdpDiscovery.kt @@ -0,0 +1,126 @@ +package com.fabledsword.minstrel.player.output.upnp + +import android.content.Context +import android.net.wifi.WifiManager +import androidx.core.content.getSystemService +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job +import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.SharedFlow +import kotlinx.coroutines.flow.asSharedFlow +import kotlinx.coroutines.isActive +import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext +import java.net.DatagramPacket +import java.net.InetAddress +import java.net.MulticastSocket +import java.nio.charset.StandardCharsets + +/** + * UDP multicast SSDP listener + M-SEARCH sender. Emits each discovery + * response's LOCATION URL on the [discoveries] SharedFlow; the + * `UpnpDiscoveryController` follows up by fetching + parsing each + * device description. + * + * Passive listen is always-on once [start] is called (NOTIFY packets + * speakers send when they boot / refresh). Active discovery (an + * explicit M-SEARCH request) is triggered by [requestActiveScan] — + * called when the picker sheet opens so newly-paired devices appear + * promptly. + * + * WifiManager.MulticastLock is held while the listener is running. + * Released by [stop] / cancellation of the parent scope. + */ +class SsdpDiscovery( + private val context: Context, +) { + private val discoveriesInternal = + MutableSharedFlow(extraBufferCapacity = DISCOVERY_BUFFER_CAPACITY) + val discoveries: SharedFlow = discoveriesInternal.asSharedFlow() + + private var multicastLock: WifiManager.MulticastLock? = null + private var socket: MulticastSocket? = null + private var listenJob: Job? = null + + fun start(scope: CoroutineScope) { + if (listenJob != null) return + val wifi = context.getSystemService() ?: return + multicastLock = wifi.createMulticastLock(MULTICAST_LOCK_TAG).apply { + setReferenceCounted(false) + acquire() + } + val sock = MulticastSocket(ANY_LOCAL_PORT).apply { + joinGroup(InetAddress.getByName(SSDP_MULTICAST_ADDR)) + } + socket = sock + listenJob = scope.launch(Dispatchers.IO) { + val buf = ByteArray(SOCKET_READ_BUFFER_BYTES) + val packet = DatagramPacket(buf, buf.size) + while (isActive) { + runCatching { sock.receive(packet) }.onSuccess { + val raw = String(packet.data, 0, packet.length, StandardCharsets.UTF_8) + parseLocation(raw)?.let { discoveriesInternal.tryEmit(it) } + } + } + } + } + + fun stop() { + listenJob?.cancel() + listenJob = null + runCatching { socket?.close() } + socket = null + runCatching { multicastLock?.release() } + multicastLock = null + } + + /** + * Send an M-SEARCH packet asking for MediaRenderer:1 devices. + * Responses arrive on the listener socket and emit via + * [discoveries] as their LOCATION URL. + */ + suspend fun requestActiveScan() = withContext(Dispatchers.IO) { + val sock = socket ?: return@withContext + val payload = buildMSearchPayload().toByteArray(StandardCharsets.UTF_8) + val packet = DatagramPacket( + payload, + payload.size, + InetAddress.getByName(SSDP_MULTICAST_ADDR), + SSDP_PORT, + ) + runCatching { sock.send(packet) } + Unit + } + + private fun buildMSearchPayload(): String = buildString { + append("M-SEARCH * HTTP/1.1\r\n") + append("HOST: ").append(SSDP_MULTICAST_ADDR).append(":").append(SSDP_PORT).append("\r\n") + append("MAN: \"ssdp:discover\"\r\n") + append("MX: ").append(MSEARCH_MX_SECONDS).append("\r\n") + append("ST: ").append(MEDIA_RENDERER_TARGET).append("\r\n") + append("\r\n") + } + + private fun parseLocation(raw: String): String? { + raw.lineSequence().forEach { line -> + val trimmed = line.trim() + if (trimmed.startsWith(LOCATION_HEADER, ignoreCase = true)) { + return trimmed.substring(LOCATION_HEADER.length).trim() + } + } + return null + } + + private companion object { + const val SSDP_MULTICAST_ADDR = "239.255.255.250" + const val SSDP_PORT = 1900 + const val ANY_LOCAL_PORT = 0 + const val SOCKET_READ_BUFFER_BYTES = 4096 + const val DISCOVERY_BUFFER_CAPACITY = 32 + const val MSEARCH_MX_SECONDS = 2 + const val MULTICAST_LOCK_TAG = "minstrel.upnp.ssdp" + const val MEDIA_RENDERER_TARGET = "urn:schemas-upnp-org:device:MediaRenderer:1" + const val LOCATION_HEADER = "LOCATION:" + } +} diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/UpnpDiscoveryController.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/UpnpDiscoveryController.kt new file mode 100644 index 00000000..ad0a8ebb --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/UpnpDiscoveryController.kt @@ -0,0 +1,126 @@ +package com.fabledsword.minstrel.player.output.upnp + +import android.content.Context +import com.fabledsword.minstrel.di.ApplicationScope +import dagger.hilt.android.qualifiers.ApplicationContext +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.launch +import okhttp3.HttpUrl +import okhttp3.HttpUrl.Companion.toHttpUrlOrNull +import okhttp3.OkHttpClient +import okhttp3.Request +import javax.inject.Inject +import javax.inject.Singleton + +/** + * Owns the SSDP listener lifecycle, fetches each discovered LOCATION's + * device description XML, and projects discovered MediaRenderers into a + * `StateFlow>` that [OutputPickerController] merges with + * system routes. + * + * Passive listen runs from process start (via `init`). Active discovery + * (an M-SEARCH burst) is triggered by [upgradeDiscovery] when the + * picker sheet opens; SSDP has no symmetric "downgrade" — the passive + * NOTIFY listener stays on for the process lifetime, so + * [downgradeDiscovery] is a no-op preserved for API parity with the + * MediaRouter-side controller. + * + * Discovered devices are de-duplicated by UDN: a repeated NOTIFY for + * the same speaker replaces the prior entry rather than appending a + * duplicate row to the picker. + */ +@Singleton +class UpnpDiscoveryController @Inject constructor( + @ApplicationContext context: Context, + @ApplicationScope private val appScope: CoroutineScope, + private val okHttp: OkHttpClient, +) { + private val ssdp = SsdpDiscovery(context) + + private val routesInternal = MutableStateFlow>(emptyList()) + val routes: StateFlow> = routesInternal.asStateFlow() + + init { + ssdp.start(appScope) + // appScope is process-lifetime (SupervisorJob + Dispatchers.Default), + // so the launched collector dies with the process — no explicit + // cancellation needed. + appScope.launch(Dispatchers.IO) { + ssdp.discoveries.collect { locationUrl -> handleDiscovery(locationUrl) } + } + } + + /** + * Fire an M-SEARCH burst so newly-paired speakers appear within a + * few seconds rather than waiting for the next NOTIFY beacon + * (typical SSDP cadence: every ~30min — far too slow for a UI + * that just opened). + */ + fun upgradeDiscovery() { + appScope.launch { ssdp.requestActiveScan() } + } + + /** + * No-op for the SSDP socket — passive NOTIFY listen is always on + * once [init] has run. Kept symmetric with the MediaRouter side's + * `downgradeDiscovery` so the picker controller fans out to both + * without conditional logic. + */ + fun downgradeDiscovery() { + // intentionally empty: see kdoc + } + + /** + * Build an [AVTransportClient] bound to the previously-discovered + * route's control URL. Returns null when the routeId isn't in the + * current snapshot — the speaker disappeared between picker open + * and tap, or the caller passed a non-UPnP routeId. Callers handle + * the null by abandoning the selection (no crash, no fallback). + */ + fun transportFor(routeId: String): AVTransportClient? { + val route = routesInternal.value.firstOrNull { it.id == routeId } ?: return null + return AVTransportClient(SoapClient(okHttp), route.avTransportControlUrl) + } + + private suspend fun handleDiscovery(locationUrl: String) { + val route = fetchRoute(locationUrl) ?: return + routesInternal.value = + routesInternal.value.filterNot { it.id == route.id } + route + } + + /** + * Fetch + parse the device description at [locationUrl] into a + * [UpnpRoute]. Returns null on URL parse failure, transport + * failure, empty body, or non-renderer device. Single-return form + * (chained `let`s + an early null guard) so detekt's default + * ReturnCount cap is respected. + */ + private fun fetchRoute(locationUrl: String): UpnpRoute? { + val url = locationUrl.toHttpUrlOrNull() ?: return null + return fetchBody(url) + ?.let { DeviceDescription.parse(it, url) } + ?.let { desc -> + UpnpRoute( + id = desc.udn, + name = desc.friendlyName.ifBlank { "Network speaker" }, + manufacturer = desc.manufacturer, + modelName = desc.modelName, + avTransportControlUrl = desc.avTransportControlUrl, + renderingControlUrl = desc.renderingControlUrl, + ) + } + } + + private fun fetchBody(url: HttpUrl): String? { + val body = runCatching { + okHttp.newCall(Request.Builder().url(url).build()).execute().use { + it.body?.string() + } + }.getOrNull().orEmpty() + return body.ifEmpty { null } + } +} diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/UpnpRoute.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/UpnpRoute.kt new file mode 100644 index 00000000..029d300b --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/UpnpRoute.kt @@ -0,0 +1,27 @@ +package com.fabledsword.minstrel.player.output.upnp + +import okhttp3.HttpUrl + +/** + * A discovered UPnP / DLNA MediaRenderer (Sonos, Yamaha MusicCast, + * Bose SoundTouch, generic DLNA renderers). Lifted out of the SOAP / + * SSDP details so the picker UI consumes a narrow domain shape. + * + * Generic UPnP only for THIS slice — Sonos-specific grouping value-adds + * (group join/leave, zone topology) live in a separate Sonos extension + * scoped in + * docs/superpowers/specs/2026-06-03-android-output-picker-upnp-scope.md. + * + * [id] is the device UDN (e.g. `uuid:RINCON_ABC...`). [name] is the + * raw `` straight from the device description — callers + * fall back to "Network speaker" upstream when it's blank; this class + * does not perform that substitution itself. + */ +data class UpnpRoute( + val id: String, + val name: String, + val manufacturer: String, + val modelName: String, + val avTransportControlUrl: HttpUrl, + val renderingControlUrl: HttpUrl?, +) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/ui/NowPlayingScreen.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/ui/NowPlayingScreen.kt index 8002842b..4115a767 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/ui/NowPlayingScreen.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/ui/NowPlayingScreen.kt @@ -2,6 +2,10 @@ package com.fabledsword.minstrel.player.ui +import android.Manifest +import android.content.pm.PackageManager +import androidx.activity.compose.rememberLauncherForActivityResult +import androidx.activity.result.contract.ActivityResultContracts import androidx.compose.animation.ExperimentalSharedTransitionApi import androidx.compose.foundation.background import androidx.compose.foundation.rememberScrollState @@ -44,13 +48,17 @@ import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip +import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.dp +import androidx.core.content.ContextCompat import androidx.hilt.navigation.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.navigation.NavHostController @@ -66,6 +74,11 @@ import com.composables.icons.lucide.Shuffle import com.composables.icons.lucide.SkipBack import com.composables.icons.lucide.SkipForward import com.fabledsword.minstrel.player.RepeatMode +import com.fabledsword.minstrel.player.output.DeviceChip +import com.fabledsword.minstrel.player.output.OutputPickerSheet +import com.fabledsword.minstrel.player.output.OutputPickerViewModel +import com.fabledsword.minstrel.player.output.OutputRoute +import com.fabledsword.minstrel.player.output.RouteSnapshot import com.fabledsword.minstrel.nav.AlbumDetail import com.fabledsword.minstrel.nav.ArtistDetail import com.fabledsword.minstrel.nav.HERO_KEY_NOW_PLAYING_COVER @@ -88,6 +101,8 @@ private const val PLAY_PAUSE_ICON_DP = 56 private const val POP_GRACE_MS = 500L private val SCRUB_TRACK_HEIGHT_DP = 4.dp private val SCRUB_TRACK_CORNER_DP = 2.dp +private val SCRUB_THUMB_WIDTH_DP = 4.dp +private val SCRUB_THUMB_HEIGHT_DP = 18.dp // Vertical drag-down threshold (in pixels) past which the gesture // pops the player. Matches Flutter's 80px threshold in spirit; @@ -266,6 +281,44 @@ private fun NowPlayingBody( ) { val isLiked by trackActionsViewModel.isLikedFlow(track.id) .collectAsStateWithLifecycle(initialValue = false) + val outputViewModel: OutputPickerViewModel = hiltViewModel() + val routes by outputViewModel.routes.collectAsStateWithLifecycle() + val sheetVisible by outputViewModel.sheetVisible.collectAsStateWithLifecycle() + val permissionDenied = rememberBluetoothPermissionState(sheetVisible) + NowPlayingContent( + inner = inner, + state = state, + track = track, + navController = navController, + viewModel = viewModel, + trackActionsViewModel = trackActionsViewModel, + routes = routes, + isLiked = isLiked, + onChipTapped = outputViewModel::onChipTapped, + ) + if (sheetVisible) { + OutputPickerSheet( + snapshot = routes, + permissionDenied = permissionDenied, + onRouteSelected = outputViewModel::onRouteSelected, + onDismiss = outputViewModel::onSheetDismissed, + ) + } +} + +@Composable +@Suppress("LongParameterList") // mirrors NowPlayingBody — pure layout wiring +private fun NowPlayingContent( + inner: androidx.compose.foundation.layout.PaddingValues, + state: com.fabledsword.minstrel.player.PlayerUiState, + track: com.fabledsword.minstrel.models.TrackRef, + navController: NavHostController, + viewModel: PlayerViewModel, + trackActionsViewModel: TrackActionsViewModel, + routes: RouteSnapshot, + isLiked: Boolean, + onChipTapped: () -> Unit, +) { Column( modifier = Modifier .fillMaxSize() @@ -293,27 +346,93 @@ private fun NowPlayingBody( onToggleShuffle = viewModel::toggleShuffle, onCycleRepeat = viewModel::cycleRepeat, ) + // Spec visibility rule: hide the chip when the only route is the + // built-in speaker — no picker is useful with one option. Chip + // reappears the moment a Bluetooth pair or wired plug arrives. + if (shouldShowChip(routes)) { + Spacer(Modifier.height(8.dp)) + DeviceChip( + route = routes.current, + onClick = onChipTapped, + modifier = Modifier.align(Alignment.CenterHorizontally), + ) + } Spacer(Modifier.height(4.dp)) - val smoothPositionMs by rememberSmoothPositionMs( - positionMs = state.positionMs, - durationMs = state.durationMs, - isPlaying = state.isPlaying, - ) - ScrubberRow( - positionMs = smoothPositionMs, - durationMs = state.durationMs, - onSeek = viewModel::seekTo, - ) - Spacer(Modifier.height(4.dp)) - TransportRow( - isPlaying = state.isPlaying, - onPrev = viewModel::skipToPrevious, - onPlayPause = { if (state.isPlaying) viewModel.pause() else viewModel.play() }, - onNext = viewModel::skipToNext, - ) + PlaybackControlsBlock(state = state, viewModel = viewModel) } } +/** + * Scrubber + transport pair, sharing the smoothed playback position. + * Extracted from [NowPlayingContent] to keep that body under detekt's + * LongMethod ceiling — the two rows belong together (the scrubber's + * smoothed position would otherwise need to be hoisted into the + * caller just to thread it into the row below). + */ +@Composable +private fun PlaybackControlsBlock( + state: com.fabledsword.minstrel.player.PlayerUiState, + viewModel: PlayerViewModel, +) { + val smoothPositionMs by rememberSmoothPositionMs( + positionMs = state.positionMs, + durationMs = state.durationMs, + isPlaying = state.isPlaying, + ) + ScrubberRow( + positionMs = smoothPositionMs, + durationMs = state.durationMs, + onSeek = viewModel::seekTo, + ) + Spacer(Modifier.height(4.dp)) + TransportRow( + isPlaying = state.isPlaying, + onPrev = viewModel::skipToPrevious, + onPlayPause = { if (state.isPlaying) viewModel.pause() else viewModel.play() }, + onNext = viewModel::skipToNext, + ) +} + +/** + * One-shot BLUETOOTH_CONNECT permission flow tied to picker-sheet + * visibility. The launcher is remembered across recompositions; the + * LaunchedEffect fires when the sheet opens and the permission is not + * already granted. Subsequent opens are no-ops once the user has + * answered — the system remembers their choice. Returns whether the + * user explicitly denied, so the sheet can surface the rationale row. + * + * Extracted from [NowPlayingBody] to keep that body under detekt's + * LongMethod ceiling — the permission plumbing is incidental to the + * player layout. + */ +@Composable +private fun rememberBluetoothPermissionState(sheetVisible: Boolean): Boolean { + val context = LocalContext.current + var permissionDenied by remember { mutableStateOf(false) } + val permissionLauncher = rememberLauncherForActivityResult( + contract = ActivityResultContracts.RequestPermission(), + ) { granted -> + permissionDenied = !granted + } + LaunchedEffect(sheetVisible) { + if (sheetVisible && + ContextCompat.checkSelfPermission( + context, + Manifest.permission.BLUETOOTH_CONNECT, + ) != PackageManager.PERMISSION_GRANTED + ) { + permissionLauncher.launch(Manifest.permission.BLUETOOTH_CONNECT) + } + } + return permissionDenied +} + +private fun shouldShowChip(snapshot: RouteSnapshot): Boolean { + val onlyBuiltIn = snapshot.available.size == 1 && + snapshot.available.first().kind == OutputRoute.Kind.BuiltIn + return !onlyBuiltIn +} + @Composable private fun BottomActionsRow( navController: NavHostController, @@ -478,15 +597,22 @@ private fun ScrubberRow(positionMs: Long, durationMs: Long, onSeek: (Long) -> Un modifier = Modifier.fillMaxWidth(), colors = sliderColors, interactionSource = interactionSource, - // Plain filled circle to match the web client's `` thumb — flat, no state-layer - // ring, no border. M3's SliderDefaults.Thumb paints a state - // layer halo on press; we drop it for visual parity. Slider's - // 48dp hit slop still applies, so tapability is unchanged. + // Thin vertical pill (4dp wide × 18dp tall, fully rounded). + // A small circle on a thin horizontal bar reads as visually + // off-center even when geometrically aligned — the eye + // expects the bar to bisect the thumb but a 14dp circle's + // mass extends above and below in equal amounts that the + // brain perceives as offset. A vertical pill the same width + // as the track removes the ambiguity: the bar passes through + // the pill's horizontal axis cleanly. Also matches M3's + // expressive-slider handle direction. State-layer halo is + // still dropped for visual parity with the web scrubber. + // Slider's 48dp hit slop still applies, so tapability is + // unchanged. thumb = { Box( modifier = Modifier - .size(14.dp) + .size(width = SCRUB_THUMB_WIDTH_DP, height = SCRUB_THUMB_HEIGHT_DP) .clip(CircleShape) .background(accent), ) @@ -513,12 +639,11 @@ private fun ScrubberRow(positionMs: Long, durationMs: Long, onSeek: (Long) -> Un /** * Custom 4dp rounded scrubber track. M3's default Track is 16dp tall - * and reads as a heavy pill rather than a measurement line; making - * the thumb visibly taller than the track (14dp thumb on a 4dp bar) - * restores the "handle on a string" cue your eye reads as "tool, - * draggable." Also drops M3's stop indicator dot, which the web - * scrubber doesn't have. Extracted so [ScrubberRow] stays under - * detekt's LongMethod ceiling. + * and reads as a heavy pill rather than a measurement line; the + * pill-thumb-on-thin-track pairing restores the "handle on a string" + * cue your eye reads as "tool, draggable." Also drops M3's stop + * indicator dot, which the web scrubber doesn't have. Extracted so + * [ScrubberRow] stays under detekt's LongMethod ceiling. */ @Composable private fun ScrubTrack(fraction: Float, accent: Color) { diff --git a/android/app/src/main/java/com/fabledsword/minstrel/playlists/data/PlaylistsRepository.kt b/android/app/src/main/java/com/fabledsword/minstrel/playlists/data/PlaylistsRepository.kt index 3f744ef2..02bf1c26 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/playlists/data/PlaylistsRepository.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/playlists/data/PlaylistsRepository.kt @@ -16,6 +16,7 @@ import com.fabledsword.minstrel.models.TrackRef import com.fabledsword.minstrel.models.wire.PlaylistDetailWire import com.fabledsword.minstrel.models.wire.PlaylistTrackWire import com.fabledsword.minstrel.models.wire.PlaylistWire +import com.fabledsword.minstrel.shared.resolveServerUrl import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.map import retrofit2.Retrofit @@ -241,7 +242,11 @@ private fun CachedPlaylistEntity.toDomain(): PlaylistRef = isPublic = isPublic, systemVariant = systemVariant, trackCount = trackCount, - coverUrl = coverPath ?: "", + // Server returns a relative cover path like "/api/playlists//cover"; + // wrap through the placeholder host so BaseUrlInterceptor + the shared + // Coil/OkHttpClient resolve it against the live server. Without this + // wrap AsyncImage silently fails on the relative URL. + coverUrl = resolveServerUrl(coverPath) ?: "", ) private fun PlaylistWire.toEntity(): CachedPlaylistEntity = @@ -277,7 +282,10 @@ private fun PlaylistDetailWire.toPlaylistRef(): PlaylistRef = isPublic = isPublic, systemVariant = systemVariant, trackCount = trackCount, - coverUrl = coverUrl, + // Same relative-path wrap as CachedPlaylistEntity.toDomain — the wire + // gives us /api/playlists//cover and Coil needs the placeholder + // host for BaseUrlInterceptor to rewrite to the live server. + coverUrl = resolveServerUrl(coverUrl) ?: "", ownerUsername = ownerUsername, ) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/playlists/ui/PlaylistDetailScreen.kt b/android/app/src/main/java/com/fabledsword/minstrel/playlists/ui/PlaylistDetailScreen.kt index db8c3529..1df819ef 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/playlists/ui/PlaylistDetailScreen.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/playlists/ui/PlaylistDetailScreen.kt @@ -87,7 +87,6 @@ import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.filter -import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch @@ -141,8 +140,7 @@ class PlaylistDetailViewModel @Inject constructor( val regenerated: Flow = regeneratedChannel.receiveAsFlow() val likedTrackIds: StateFlow> = - likes.observeLikedTracks() - .map { tracks -> tracks.mapTo(mutableSetOf()) { it.id } } + likes.observeLikedTrackIds() .stateIn( scope = viewModelScope, started = SharingStarted.WhileSubscribed(SHARE_STOP_TIMEOUT_MS), diff --git a/android/app/src/test/java/com/fabledsword/minstrel/player/output/upnp/DeviceDescriptionTest.kt b/android/app/src/test/java/com/fabledsword/minstrel/player/output/upnp/DeviceDescriptionTest.kt new file mode 100644 index 00000000..e633415c --- /dev/null +++ b/android/app/src/test/java/com/fabledsword/minstrel/player/output/upnp/DeviceDescriptionTest.kt @@ -0,0 +1,108 @@ +package com.fabledsword.minstrel.player.output.upnp + +import okhttp3.HttpUrl.Companion.toHttpUrl +import org.junit.jupiter.api.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull + +/** + * Unit tests for the UPnP device-description pull-parser. + * + * Android's stock `XmlPullParserFactory.newInstance()` resolves to a + * real impl on-device but resolves to the android.jar stub class on + * JVM unit tests. kxml2 is added as a testImplementation dep so + * `XmlPullParserFactory.newInstance()` finds it via service-provider + * lookup on the test classpath; the tests run unconditionally. + */ +class DeviceDescriptionTest { + + private val base = "http://192.168.1.50:1400/xml/device_description.xml".toHttpUrl() + + @Test + fun `parses Sonos-shaped description`() { + val xml = """ + + + + urn:schemas-upnp-org:device:MediaRenderer:1 + Living Room + Sonos, Inc. + Sonos One + uuid:RINCON_ABC + + + urn:schemas-upnp-org:service:AVTransport:1 + /MediaRenderer/AVTransport/Control + + + urn:schemas-upnp-org:service:RenderingControl:1 + /MediaRenderer/RenderingControl/Control + + + + + """.trimIndent() + + val desc = DeviceDescription.parse(xml, base) + assertNotNull(desc) + assertEquals("Living Room", desc.friendlyName) + assertEquals("Sonos, Inc.", desc.manufacturer) + assertEquals("Sonos One", desc.modelName) + assertEquals("uuid:RINCON_ABC", desc.udn) + assertEquals( + "http://192.168.1.50:1400/MediaRenderer/AVTransport/Control", + desc.avTransportControlUrl.toString(), + ) + assertEquals( + "http://192.168.1.50:1400/MediaRenderer/RenderingControl/Control", + desc.renderingControlUrl?.toString(), + ) + } + + @Test + fun `drops device with no AVTransport service`() { + val xml = """ + + + + Stub TV + uuid:STUB + + + urn:schemas-upnp-org:service:ConnectionManager:1 + /cm + + + + + """.trimIndent() + + assertNull(DeviceDescription.parse(xml, base)) + } + + @Test + fun `handles missing optional fields`() { + val xml = """ + + + + uuid:MIN + + + urn:schemas-upnp-org:service:AVTransport:1 + /avt + + + + + """.trimIndent() + + val desc = DeviceDescription.parse(xml, base) + assertNotNull(desc) + assertEquals("", desc.friendlyName) + assertEquals("", desc.manufacturer) + assertEquals("", desc.modelName) + assertNull(desc.renderingControlUrl) + } +} diff --git a/android/app/src/test/java/com/fabledsword/minstrel/player/output/upnp/SoapClientTest.kt b/android/app/src/test/java/com/fabledsword/minstrel/player/output/upnp/SoapClientTest.kt new file mode 100644 index 00000000..ad052e72 --- /dev/null +++ b/android/app/src/test/java/com/fabledsword/minstrel/player/output/upnp/SoapClientTest.kt @@ -0,0 +1,146 @@ +package com.fabledsword.minstrel.player.output.upnp + +import kotlinx.coroutines.test.runTest +import okhttp3.OkHttpClient +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertTrue + +/** + * Unit tests for SoapClient. Uses MockWebServer to capture the + * outgoing SOAP envelope + verify the SOAPACTION header shape, and + * to drive the `` response path. + * + * kxml2 is on the test classpath (Task 4), so + * `XmlPullParserFactory.newInstance()` resolves on the JVM. + */ +class SoapClientTest { + private lateinit var server: MockWebServer + private lateinit var client: SoapClient + + @BeforeEach + fun setup() { + server = MockWebServer().apply { start() } + client = SoapClient(OkHttpClient()) + } + + @AfterEach + fun teardown() { + server.shutdown() + } + + @Test + fun `SetAVTransportURI envelope sent with correct SOAPACTION header`() = runTest { + server.enqueue( + MockResponse() + .setBody(setUriResponseBody()) + .setHeader("Content-Type", "text/xml"), + ) + + client.call( + controlUrl = server.url("/avt/Control"), + serviceType = "urn:schemas-upnp-org:service:AVTransport:1", + action = "SetAVTransportURI", + args = mapOf( + "InstanceID" to "0", + "CurrentURI" to "http://server/api/tracks/x/stream?token=t&exp=1", + "CurrentURIMetaData" to "", + ), + ) + + val recorded = server.takeRequest() + assertEquals( + "\"urn:schemas-upnp-org:service:AVTransport:1#SetAVTransportURI\"", + recorded.getHeader("SOAPACTION"), + ) + val body = recorded.body.readUtf8() + assertTrue( + body.contains("http://server/api/tracks/x/stream?token=t&exp=1", + ), + "envelope missing escaped CurrentURI: $body", + ) + } + + @Test + fun `XML special chars in args are escaped`() = runTest { + server.enqueue( + MockResponse() + .setBody(setUriResponseBody()) + .setHeader("Content-Type", "text/xml"), + ) + + client.call( + controlUrl = server.url("/avt/Control"), + serviceType = "urn:schemas-upnp-org:service:AVTransport:1", + action = "SetAVTransportURI", + args = mapOf("CurrentURI" to "a&b\"d'e"), + ) + + val recorded = server.takeRequest() + val body = recorded.body.readUtf8() + assertTrue( + body.contains("a&b<c>"d'e"), + "expected all five XML escapes in body, got: $body", + ) + } + + @Test + fun `SOAP fault becomes SoapFaultException`() = runTest { + server.enqueue( + MockResponse() + .setResponseCode(500) + .setBody(faultResponseBody()) + .setHeader("Content-Type", "text/xml"), + ) + + val ex = assertFailsWith { + client.call( + controlUrl = server.url("/avt/Control"), + serviceType = "urn:schemas-upnp-org:service:AVTransport:1", + action = "Play", + args = mapOf("InstanceID" to "0", "Speed" to "1"), + ) + } + assertEquals("402", ex.code) + } + + private fun setUriResponseBody(): String = """ + + + + + + + """.trimIndent() + + private fun faultResponseBody(): String = """ + + + + + s:Client + UPnPError + + + 402 + Invalid Args + + + + + + """.trimIndent() +} diff --git a/android/gradle/libs.versions.toml b/android/gradle/libs.versions.toml index 8a2be34f..5165ea49 100644 --- a/android/gradle/libs.versions.toml +++ b/android/gradle/libs.versions.toml @@ -22,6 +22,7 @@ kotlinx-coroutines = "1.9.0" kotlinx-datetime = "0.6.1" kotlinx-serialization-converter = "1.0.0" media3 = "1.10.1" +mediarouter = "1.7.0" coil = "3.0.0-rc02" palette = "1.0.0" timber = "5.0.1" @@ -33,6 +34,7 @@ mockk = "1.13.13" compose-test = "1.7.5" ktlint-gradle = "12.1.1" detekt = "2.0.0-alpha.3" +kxml2 = "2.3.0" [libraries] androidx-core-ktx = { module = "androidx.core:core-ktx", version = "1.13.1" } @@ -71,6 +73,7 @@ media3-exoplayer = { module = "androidx.media3:media3-exoplayer", version.ref = media3-session = { module = "androidx.media3:media3-session", version.ref = "media3" } media3-datasource-okhttp = { module = "androidx.media3:media3-datasource-okhttp", version.ref = "media3" } media3-ui = { module = "androidx.media3:media3-ui", version.ref = "media3" } +mediarouter = { module = "androidx.mediarouter:mediarouter", version.ref = "mediarouter" } coil-compose = { module = "io.coil-kt.coil3:coil-compose", version.ref = "coil" } coil-network-okhttp = { module = "io.coil-kt.coil3:coil-network-okhttp", version.ref = "coil" } androidx-palette = { module = "androidx.palette:palette-ktx", version.ref = "palette" } @@ -87,6 +90,7 @@ turbine = { module = "app.cash.turbine:turbine", version.ref = "turbine" } mockk = { module = "io.mockk:mockk", version.ref = "mockk" } compose-ui-test = { module = "androidx.compose.ui:ui-test-junit4" } compose-ui-test-manifest = { module = "androidx.compose.ui:ui-test-manifest" } +kxml2 = { module = "net.sf.kxml:kxml2", version.ref = "kxml2" } [plugins] android-application = { id = "com.android.application", version.ref = "agp" } diff --git a/cmd/minstrel/main.go b/cmd/minstrel/main.go index 04cfe6ff..285f00ff 100644 --- a/cmd/minstrel/main.go +++ b/cmd/minstrel/main.go @@ -245,6 +245,7 @@ func run() error { }, cfg.Events, cfg.Recommendation, cfg.Storage.DataDir, cfg.Branding, coverEnricher, coverSettings, scanner, scanCfg, scheduler) srv.Bus = bus srv.PlaylistScheduler = playlistScheduler + srv.StreamSecret = cfg.StreamSecret httpServer := &http.Server{ Addr: cfg.Server.Address, Handler: srv.Router(), diff --git a/internal/api/api.go b/internal/api/api.go index 1912eece..4a644661 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -28,7 +28,7 @@ import ( // Mount attaches /api/* handlers to r. Public endpoints (login) are outside // RequireUser; everything else is gated by the middleware. The events writer // is shared with the Subsonic mount so /rest/scrobble feeds the same store. -func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playevents.Writer, recCfg config.RecommendationConfig, lidarrCfg *lidarrconfig.Service, lidarrReqs *lidarrrequests.Service, lidarrQuar *lidarrquarantine.Service, tracksSvc *tracks.Service, playlistsSvc *playlists.Service, coverEnricher *coverart.Enricher, coverSettings *coverart.SettingsService, scanner *library.Scanner, scanCfg library.RunScanConfig, scheduler *library.Scheduler, dataDir string, sender mailer.Sender, bus *eventbus.Bus, playlistScheduler *playlists.Scheduler) { +func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playevents.Writer, recCfg config.RecommendationConfig, lidarrCfg *lidarrconfig.Service, lidarrReqs *lidarrrequests.Service, lidarrQuar *lidarrquarantine.Service, tracksSvc *tracks.Service, playlistsSvc *playlists.Service, coverEnricher *coverart.Enricher, coverSettings *coverart.SettingsService, scanner *library.Scanner, scanCfg library.RunScanConfig, scheduler *library.Scheduler, dataDir string, sender mailer.Sender, bus *eventbus.Bus, playlistScheduler *playlists.Scheduler, streamSecret []byte) { rng := rand.New(rand.NewSource(rand.Int63())) h := &handlers{ pool: pool, logger: logger, events: events, recCfg: recCfg, @@ -47,6 +47,7 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev mailer: sender, eventbus: bus, playlistScheduler: playlistScheduler, + streamSecret: streamSecret, } r.Route("/api", func(api chi.Router) { @@ -54,6 +55,17 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev api.Post("/auth/register", h.handleRegister) api.Post("/auth/forgot-password", h.handleForgotPassword) api.Post("/auth/reset-password", h.handleResetPassword) + + // Stream lives outside authed.Group so it can accept EITHER a + // session (resolved by the OptionalUser middleware) OR a signed + // query token (UPnP / Sonos path; see streamAuthOk). The + // middleware attaches user to context when a valid cookie / + // bearer is present but does NOT 401 on absence; the handler's + // own streamAuthOk performs the actual auth check. See the + // design at + // docs/superpowers/specs/2026-06-03-android-output-picker-upnp-design.md. + api.With(auth.OptionalUser(pool, logger)).Get("/tracks/{id}/stream", h.handleGetStream) + api.Group(func(authed chi.Router) { authed.Use(auth.RequireUser(pool)) authed.Post("/auth/logout", h.handleLogout) @@ -77,7 +89,8 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev authed.Get("/library/albums", h.handleListLibraryAlbums) authed.Get("/library/sync", h.handleLibrarySync) authed.Get("/tracks/{id}", h.handleGetTrack) - authed.Get("/tracks/{id}/stream", h.handleGetStream) + // /tracks/{id}/stream is mounted above with OptionalUser so + // it can accept either a session or a signed token. authed.Get("/search", h.handleSearch) authed.Get("/radio", h.handleRadio) authed.Get("/discover/suggestions", h.handleListSuggestions) @@ -85,6 +98,11 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev authed.Get("/home/index", h.handleGetHomeIndex) authed.Post("/events", h.handleEvents) authed.Get("/events/stream", h.handleEventsStream) + // UPnP / Sonos cast slice: issue a short-lived HMAC stream URL + // the speaker can fetch without the user's session. See the + // design at + // docs/superpowers/specs/2026-06-03-android-output-picker-upnp-design.md. + authed.Post("/cast/stream-token", h.handleCastStreamToken) authed.Post("/likes/tracks/{id}", h.handleLikeTrack) authed.Delete("/likes/tracks/{id}", h.handleUnlikeTrack) authed.Post("/likes/albums/{id}", h.handleLikeAlbum) @@ -205,4 +223,13 @@ type handlers struct { mailer mailer.Sender eventbus *eventbus.Bus playlistScheduler *playlists.Scheduler + // streamSecret is the HMAC key used by SignStreamToken / + // VerifyStreamToken to authenticate the UPnP-speaker stream path + // (see internal/api/stream_token.go and the design at + // docs/superpowers/specs/2026-06-03-android-output-picker-upnp-design.md). + // nil in slice 1; slice 2 wires the env-var-with-app_preferences- + // fallback loader. A nil secret leaves the cookie path intact and + // makes the token path unreachable (HMAC of empty key won't match + // anything a client mints), which is the desired slice-1 default. + streamSecret []byte } diff --git a/internal/api/cast_token.go b/internal/api/cast_token.go new file mode 100644 index 00000000..5fdf14f8 --- /dev/null +++ b/internal/api/cast_token.go @@ -0,0 +1,83 @@ +package api + +import ( + "net/http" + "strconv" + "time" + + "git.fabledsword.com/bvandeusen/minstrel/internal/apierror" +) + +const ( + castTokenMinExpSeconds = 60 + castTokenMaxExpSeconds = 86400 // 24h + castTokenDefaultExp = 21600 // 6h +) + +type castTokenRequest struct { + TrackID string `json:"trackId"` + ExpSeconds int `json:"expSeconds,omitempty"` +} + +type castTokenResponse struct { + Token string `json:"token"` + Exp int64 `json:"exp"` + URL string `json:"url"` +} + +// handleCastStreamToken issues a short-lived HMAC stream token for the +// given trackId. Authenticated via the standard session cookie / bearer. +// +// The returned URL is a fully-formed stream URL (token + exp embedded +// as query params) that the client passes verbatim to a UPnP / Sonos +// device's AVTransport.SetAVTransportURI call — those devices cannot +// carry the user's session, so the signed query string is the only way +// they can fetch the bytes. +// +// expSeconds is clamped to [60, 86400]; default 21600 (6h) — long enough +// to play through any typical track without re-minting mid-playback. +// +// Part of the output-picker UPnP slice. See +// docs/superpowers/specs/2026-06-03-android-output-picker-upnp-design.md. +func (h *handlers) handleCastStreamToken(w http.ResponseWriter, r *http.Request) { + if _, ok := requireUser(w, r); !ok { + return + } + var req castTokenRequest + if !decodeBody(w, r, &req) { + return + } + trackUUID, ok := parseUUID(req.TrackID) + if !ok || !trackUUID.Valid { + writeErr(w, apierror.BadRequest("invalid_track_id", "trackId must be a UUID")) + return + } + expSec := clampExpSeconds(req.ExpSeconds) + exp := time.Now().Add(time.Duration(expSec) * time.Second).Unix() + token := SignStreamToken(h.streamSecret, req.TrackID, exp) + + scheme := "http" + if r.TLS != nil { + scheme = "https" + } + url := scheme + "://" + r.Host + "/api/tracks/" + req.TrackID + + "/stream?token=" + token + "&exp=" + strconv.FormatInt(exp, 10) + + writeJSON(w, http.StatusOK, castTokenResponse{Token: token, Exp: exp, URL: url}) +} + +// clampExpSeconds applies the [60, 86400] window with a 6h default for +// non-positive inputs. Extracted so it doesn't bloat the handler's +// detekt-equivalent line count and so the test can exercise edges directly. +func clampExpSeconds(v int) int { + if v <= 0 { + return castTokenDefaultExp + } + if v < castTokenMinExpSeconds { + return castTokenMinExpSeconds + } + if v > castTokenMaxExpSeconds { + return castTokenMaxExpSeconds + } + return v +} diff --git a/internal/api/cast_token_test.go b/internal/api/cast_token_test.go new file mode 100644 index 00000000..0687b57d --- /dev/null +++ b/internal/api/cast_token_test.go @@ -0,0 +1,152 @@ +package api + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" +) + +const testTrackUUID = "11111111-1111-1111-1111-111111111111" + +func TestCastStreamToken_HappyPath(t *testing.T) { + h, pool := testHandlers(t) + user := seedUser(t, pool, "alice", "hunter2", false) + h.streamSecret = []byte("cast-token-test-secret") + + body, err := json.Marshal(castTokenRequest{ + TrackID: testTrackUUID, + ExpSeconds: 3600, + }) + if err != nil { + t.Fatalf("marshal: %v", err) + } + req := httptest.NewRequest(http.MethodPost, "/api/cast/stream-token", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + req = withUser(req, user) + w := httptest.NewRecorder() + + h.handleCastStreamToken(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%s", w.Code, w.Body.String()) + } + var resp castTokenResponse + if err := json.NewDecoder(w.Body).Decode(&resp); err != nil { + t.Fatalf("decode: %v", err) + } + if resp.Token == "" || resp.Exp == 0 || resp.URL == "" { + t.Fatalf("empty fields in response: %+v", resp) + } + if !strings.Contains(resp.URL, "token="+resp.Token) { + t.Fatalf("URL missing token query: %s", resp.URL) + } + if !strings.Contains(resp.URL, "/api/tracks/"+testTrackUUID+"/stream") { + t.Fatalf("URL missing stream path: %s", resp.URL) + } + if !VerifyStreamToken(h.streamSecret, testTrackUUID, resp.Exp, resp.Token) { + t.Fatal("returned token does not verify") + } +} + +func TestCastStreamToken_RejectsBadUUID(t *testing.T) { + h, pool := testHandlers(t) + user := seedUser(t, pool, "alice", "hunter2", false) + h.streamSecret = []byte("cast-token-test-secret") + + body, err := json.Marshal(castTokenRequest{TrackID: "not-a-uuid"}) + if err != nil { + t.Fatalf("marshal: %v", err) + } + req := httptest.NewRequest(http.MethodPost, "/api/cast/stream-token", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + req = withUser(req, user) + w := httptest.NewRecorder() + + h.handleCastStreamToken(w, req) + + if w.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400; body=%s", w.Code, w.Body.String()) + } +} + +func TestCastStreamToken_RejectsUnauthenticated(t *testing.T) { + h, _ := testHandlers(t) + h.streamSecret = []byte("cast-token-test-secret") + + body, err := json.Marshal(castTokenRequest{TrackID: testTrackUUID}) + if err != nil { + t.Fatalf("marshal: %v", err) + } + req := httptest.NewRequest(http.MethodPost, "/api/cast/stream-token", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + // NO withUser — handler must reject via requireUser. + w := httptest.NewRecorder() + + h.handleCastStreamToken(w, req) + + if w.Code != http.StatusUnauthorized { + t.Fatalf("status = %d, want 401; body=%s", w.Code, w.Body.String()) + } +} + +func TestCastStreamToken_ClampsExpSeconds(t *testing.T) { + h, pool := testHandlers(t) + user := seedUser(t, pool, "alice", "hunter2", false) + h.streamSecret = []byte("cast-token-test-secret") + + // Request 1 second (below min 60), expect clamp to 60s. + body, err := json.Marshal(castTokenRequest{ + TrackID: testTrackUUID, + ExpSeconds: 1, + }) + if err != nil { + t.Fatalf("marshal: %v", err) + } + before := time.Now().Unix() + req := httptest.NewRequest(http.MethodPost, "/api/cast/stream-token", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + req = withUser(req, user) + w := httptest.NewRecorder() + + h.handleCastStreamToken(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%s", w.Code, w.Body.String()) + } + var resp castTokenResponse + if err := json.NewDecoder(w.Body).Decode(&resp); err != nil { + t.Fatalf("decode: %v", err) + } + // Allow a small jitter window around the clamp target (60s). + delta := resp.Exp - before + if delta < 55 || delta > 70 { + t.Fatalf("expected ~60s expiry after clamp, got delta=%ds", delta) + } +} + +func TestClampExpSeconds(t *testing.T) { + cases := []struct { + name string + in int + want int + }{ + {"zero defaults to 6h", 0, castTokenDefaultExp}, + {"negative defaults to 6h", -1, castTokenDefaultExp}, + {"below min clamps up", 30, castTokenMinExpSeconds}, + {"exact min passes through", castTokenMinExpSeconds, castTokenMinExpSeconds}, + {"mid-range passes through", 3600, 3600}, + {"exact max passes through", castTokenMaxExpSeconds, castTokenMaxExpSeconds}, + {"above max clamps down", castTokenMaxExpSeconds + 1, castTokenMaxExpSeconds}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := clampExpSeconds(tc.in); got != tc.want { + t.Fatalf("clampExpSeconds(%d) = %d, want %d", tc.in, got, tc.want) + } + }) + } +} diff --git a/internal/api/library_test.go b/internal/api/library_test.go index 845aed75..5ac1b450 100644 --- a/internal/api/library_test.go +++ b/internal/api/library_test.go @@ -12,6 +12,7 @@ import ( "github.com/go-chi/chi/v5" + "git.fabledsword.com/bvandeusen/minstrel/internal/auth" "git.fabledsword.com/bvandeusen/minstrel/internal/config" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" "git.fabledsword.com/bvandeusen/minstrel/internal/eventbus" @@ -21,7 +22,15 @@ import ( // newLibraryRouter builds a test-only chi router with the library handlers // mounted at their path-style routes. Tests hit this router rather than // calling handler methods directly so chi URL params populate correctly. -// RequireUser is NOT applied — library handlers don't read user context. +// RequireUser is NOT applied to most routes — library handlers don't read +// user context. +// +// The stream route is wrapped with a synthetic-user middleware because +// handleGetStream now enforces its own auth check (streamAuthOk) after the +// UPnP slice moved it out of the authed.Group. Real traffic carries either +// a session cookie (resolved by auth.OptionalUser) or a signed query +// token; tests get a fake user-in-context so the cookie path of +// streamAuthOk succeeds without seeding a real session row. func newLibraryRouter(h *handlers) chi.Router { r := chi.NewRouter() r.Get("/api/artists", h.handleListArtists) @@ -29,11 +38,23 @@ func newLibraryRouter(h *handlers) chi.Router { r.Get("/api/albums/{id}", h.handleGetAlbum) r.Get("/api/albums/{id}/cover", h.handleGetCover) r.Get("/api/tracks/{id}", h.handleGetTrack) - r.Get("/api/tracks/{id}/stream", h.handleGetStream) + r.With(injectFakeUserForTest).Get("/api/tracks/{id}/stream", h.handleGetStream) r.Get("/api/search", h.handleSearch) return r } +// injectFakeUserForTest attaches an empty dbq.User to request context via +// auth.UserCtxKeyForTest(), letting handleGetStream's streamAuthOk succeed +// on the session path without the test having to seed a real session row. +// Production traffic uses auth.OptionalUser instead; this is the test-only +// equivalent that bypasses the DB lookup. +func injectFakeUserForTest(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx := context.WithValue(r.Context(), auth.UserCtxKeyForTest(), dbq.User{}) + next.ServeHTTP(w, r.WithContext(ctx)) + }) +} + func TestHandleGetTrack_HappyPath(t *testing.T) { h, pool := testHandlers(t) truncateLibrary(t, pool) @@ -444,7 +465,7 @@ func TestRoutesRegisteredInMount(t *testing.T) { r := chi.NewRouter() w := playevents.NewWriter(h.pool, slog.New(slog.NewTextHandler(io.Discard, nil)), 30*time.Minute, 0.5, 30000) - Mount(r, h.pool, h.logger, w, config.RecommendationConfig{RadioSize: 50, RadioSizeMax: 200, RecentlyPlayedHours: 1}, h.lidarrCfg, h.lidarrRequests, h.lidarrQuarantine, h.tracks, h.playlists, h.coverart, h.coverSettings, h.scanner, h.scanCfg, nil, h.dataDir, nil, eventbus.New(), nil) + Mount(r, h.pool, h.logger, w, config.RecommendationConfig{RadioSize: 50, RadioSizeMax: 200, RecentlyPlayedHours: 1}, h.lidarrCfg, h.lidarrRequests, h.lidarrQuarantine, h.tracks, h.playlists, h.coverart, h.coverSettings, h.scanner, h.scanCfg, nil, h.dataDir, nil, eventbus.New(), nil, nil) paths := []string{ "/api/artists", diff --git a/internal/api/media.go b/internal/api/media.go index 0ff6cbbb..cca4afec 100644 --- a/internal/api/media.go +++ b/internal/api/media.go @@ -11,9 +11,13 @@ import ( "net/http" "os" "path/filepath" + "strconv" "strings" + "github.com/go-chi/chi/v5" + "git.fabledsword.com/bvandeusen/minstrel/internal/apierror" + "git.fabledsword.com/bvandeusen/minstrel/internal/auth" "git.fabledsword.com/bvandeusen/minstrel/internal/coverart" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" ) @@ -121,10 +125,56 @@ func (h *handlers) handleGetCover(w http.ResponseWriter, r *http.Request) { http.ServeContent(w, r, filepath.Base(path), info.ModTime(), f) } +// streamAuthOk returns true if the request is authorized to fetch the +// stream — either via the standard session path (cookie OR bearer token +// resolved to a user-in-context by auth.OptionalUser, the middleware +// the route is wrapped with in api.go) OR via a valid short-lived HMAC +// token in the ?token=&exp= query (UPnP / Sonos path, new for the +// output-picker UPnP slice). +// +// The token path lets network speakers fetch the stream URL without +// carrying the user's session cookie — they cannot. See the design at +// docs/superpowers/specs/2026-06-03-android-output-picker-upnp-design.md. +// +// When h.streamSecret is nil (slice-1 default, until slice 2 wires the +// loader), the token path always rejects because the HMAC of an empty +// key won't match any token a client could mint. The session path keeps +// working. +func (h *handlers) streamAuthOk(r *http.Request, trackID string) bool { + if _, ok := auth.UserFromContext(r.Context()); ok { + return true + } + tok := r.URL.Query().Get("token") + expStr := r.URL.Query().Get("exp") + if tok == "" || expStr == "" { + return false + } + exp, err := strconv.ParseInt(expStr, 10, 64) + if err != nil { + return false + } + return VerifyStreamToken(h.streamSecret, trackID, exp, tok) +} + // handleGetStream implements GET /api/tracks/{id}/stream. Opens the file on // disk and delegates byte-serving to http.ServeContent, which handles Range, // If-Modified-Since, and ETag based on the file's mod time. +// +// The route lives outside the authed.Group so this handler can accept +// EITHER a session-resolved user (attached by auth.OptionalUser, the +// permissive middleware the route is wrapped with) OR a signed token +// (UPnP slice). streamAuthOk gates both paths. func (h *handlers) handleGetStream(w http.ResponseWriter, r *http.Request) { + // Auth check before the DB lookup: a 404 ahead of the auth check + // would let unauth callers probe which track IDs exist via the + // 404/401 response differential. streamAuthOk is keyed on the + // path's id directly (the HMAC token is signed over the same id + // string, so we don't need the resolved row yet). + rawID := chi.URLParam(r, "id") + if !h.streamAuthOk(r, rawID) { + writeErr(w, apierror.ErrUnauthorized) + return + } track, apiErr := resolveByID(r, "id", dbq.New(h.pool).GetTrackByID, "track") if apiErr != nil { writeErr(w, apiErr) diff --git a/internal/api/stream_token.go b/internal/api/stream_token.go new file mode 100644 index 00000000..9f1d70f9 --- /dev/null +++ b/internal/api/stream_token.go @@ -0,0 +1,39 @@ +package api + +import ( + "crypto/hmac" + "crypto/sha256" + "encoding/hex" + "fmt" + "time" +) + +// SignStreamToken returns the HMAC-SHA256 hex of "|" keyed +// by secret. The token is constant-time-comparable via VerifyStreamToken +// so it's safe to expose the verification function in handlers. +// +// trackID is the canonical track UUID. exp is the Unix time after which +// the token is invalid; the handler checks exp before serving. +// +// The secret comes from MINSTREL_STREAM_SECRET (or its auto-generated + +// persisted fallback in app_preferences). The loader lands in slice 2 +// (POST /api/cast/stream-token); this file is the primitive both the +// loader and the stream handler share. +func SignStreamToken(secret []byte, trackID string, exp int64) string { + mac := hmac.New(sha256.New, secret) + // hash.Hash.Write is documented as never returning an error. + _, _ = fmt.Fprintf(mac, "%s|%d", trackID, exp) + return hex.EncodeToString(mac.Sum(nil)) +} + +// VerifyStreamToken returns true iff token is a valid HMAC for +// (trackID, exp) AND exp has not yet passed. Wall-clock comparison — +// callers must trust the server's clock. Uses hmac.Equal for +// constant-time compare so a timing oracle can't bias token guessing. +func VerifyStreamToken(secret []byte, trackID string, exp int64, token string) bool { + if time.Now().Unix() > exp { + return false + } + expected := SignStreamToken(secret, trackID, exp) + return hmac.Equal([]byte(expected), []byte(token)) +} diff --git a/internal/api/stream_token_test.go b/internal/api/stream_token_test.go new file mode 100644 index 00000000..a7e78ed9 --- /dev/null +++ b/internal/api/stream_token_test.go @@ -0,0 +1,78 @@ +package api + +import ( + "testing" + "time" +) + +func TestSignStreamToken_RoundTrip(t *testing.T) { + secret := []byte("test-secret-not-real") + trackID := "abc-123" + exp := time.Now().Unix() + 3600 + + token := SignStreamToken(secret, trackID, exp) + if token == "" { + t.Fatal("got empty token") + } + if !VerifyStreamToken(secret, trackID, exp, token) { + t.Fatal("round-trip verify failed") + } +} + +func TestVerifyStreamToken_TamperedTokenRejected(t *testing.T) { + secret := []byte("test-secret") + trackID := "abc-123" + exp := time.Now().Unix() + 3600 + + token := SignStreamToken(secret, trackID, exp) + // Flip a hex digit anywhere in the middle. + mid := len(token) / 2 + tampered := token[:mid] + flipHexDigit(token[mid:mid+1]) + token[mid+1:] + + if VerifyStreamToken(secret, trackID, exp, tampered) { + t.Fatal("verify accepted tampered token") + } +} + +func TestVerifyStreamToken_ExpiredRejected(t *testing.T) { + secret := []byte("test-secret") + trackID := "abc-123" + exp := time.Now().Unix() - 1 // already expired + + token := SignStreamToken(secret, trackID, exp) + if VerifyStreamToken(secret, trackID, exp, token) { + t.Fatal("verify accepted expired token") + } +} + +func TestVerifyStreamToken_WrongTrackIDRejected(t *testing.T) { + secret := []byte("test-secret") + exp := time.Now().Unix() + 3600 + + token := SignStreamToken(secret, "track-a", exp) + if VerifyStreamToken(secret, "track-b", exp, token) { + t.Fatal("verify accepted token for different track") + } +} + +func TestVerifyStreamToken_WrongSecretRejected(t *testing.T) { + trackID := "abc-123" + exp := time.Now().Unix() + 3600 + + token := SignStreamToken([]byte("secret-a"), trackID, exp) + if VerifyStreamToken([]byte("secret-b"), trackID, exp, token) { + t.Fatal("verify accepted token signed with different secret") + } +} + +func flipHexDigit(s string) string { + if s == "" { + return s + } + switch s[0] { + case '0': + return "1" + default: + return "0" + } +} diff --git a/internal/auth/session.go b/internal/auth/session.go index b4b12b1b..e949223e 100644 --- a/internal/auth/session.go +++ b/internal/auth/session.go @@ -112,6 +112,52 @@ func RequireUser(pool *pgxpool.Pool) func(http.Handler) http.Handler { // middleware. Do not use this outside _test.go files. func UserCtxKeyForTest() any { return userCtxKey } +// OptionalUser is RequireUser's permissive sibling: it resolves the caller +// from the session cookie or bearer header and attaches the user to context +// when present + valid, but does NOT 401 on absence. The downstream handler +// runs unconditionally and is responsible for its own auth check via +// UserFromContext (or its own bespoke path — see /api/tracks/{id}/stream's +// streamAuthOk, which accepts EITHER a user-in-context OR a signed query +// token for UPnP / Sonos speakers that don't carry the user's cookie). +// +// Invalid tokens (stale session row, deleted user) silently drop through +// without attaching the user. The handler treats "no user in context" as +// "not authenticated" the same way it treats a missing cookie. +// +// Database lookup failures fall through too — a transient DB blip should not +// 5xx a stream request that may have a perfectly valid signed token. The +// error is logged so the operator can correlate. +func OptionalUser(pool *pgxpool.Pool, logger *slog.Logger) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + token := sessionTokenFromRequest(r) + if token == "" || pool == nil { + next.ServeHTTP(w, r) + return + } + q := dbq.New(pool) + sess, err := q.GetSessionByTokenHash(r.Context(), HashSessionToken(token)) + if err != nil { + if !errors.Is(err, pgx.ErrNoRows) && logger != nil { + logger.Warn("api: optional session lookup failed", "err", err) + } + next.ServeHTTP(w, r) + return + } + user, err := q.GetUserByID(r.Context(), sess.UserID) + if err != nil { + if !errors.Is(err, pgx.ErrNoRows) && logger != nil { + logger.Warn("api: optional user lookup failed", "err", err) + } + next.ServeHTTP(w, r) + return + } + ctx := context.WithValue(r.Context(), userCtxKey, user) + next.ServeHTTP(w, r.WithContext(ctx)) + }) + } +} + func sessionTokenFromRequest(r *http.Request) string { if c, err := r.Cookie(SessionCookieName); err == nil && c.Value != "" { return c.Value diff --git a/internal/config/config.go b/internal/config/config.go index 986dad6d..e28d4432 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -1,8 +1,12 @@ package config import ( + "crypto/rand" + "encoding/base64" "fmt" + "log" "os" + "path/filepath" "strconv" "strings" @@ -19,6 +23,14 @@ type Config struct { Events EventsConfig `yaml:"events"` Recommendation RecommendationConfig `yaml:"recommendation"` Branding BrandingConfig `yaml:"branding"` + // StreamSecret is the base64-decoded HMAC key used to sign UPnP / + // Sonos stream URLs (see internal/api/stream_token.go and + // docs/superpowers/specs/2026-06-03-android-output-picker-upnp-design.md). + // Sourced from MINSTREL_STREAM_SECRET (base64-encoded), with a + // per-machine fallback persisted at /stream_secret. + // Not serialized to YAML — operator-machine-scoped runtime state, not + // a user-facing setting. Never log the contents. + StreamSecret []byte `yaml:"-"` } type ServerConfig struct { @@ -137,9 +149,83 @@ func Load(path string) (Config, error) { } } applyEnv(&cfg) + if err := resolveStreamSecret(&cfg); err != nil { + return cfg, err + } return cfg, nil } +// streamSecretBytes is the raw HMAC-key length; 64 bytes gives 512 bits +// of entropy, more than enough for HMAC-SHA256, and lands at a 86-char +// base64-url-no-padding string that fits cleanly in a single .env line. +const streamSecretBytes = 64 + +// streamSecretFile is the basename for the per-machine persisted fallback +// under . Kept as a constant so tests assert against the +// exact path and operators can find it during backup planning. +const streamSecretFile = "stream_secret" + +// resolveStreamSecret populates cfg.StreamSecret using, in order: +// 1. MINSTREL_STREAM_SECRET env var (operator-supplied, base64-url +// encoded — accepts either padded or raw form). +// 2. /stream_secret if present (auto-generated on a +// previous boot; survives restarts so signed URLs stay valid). +// 3. Auto-generate streamSecretBytes random bytes, persist them to that +// file at 0600, then use them. +// +// Logs (only) when an auto-generation happens so the operator can spot +// it during first-boot. Never logs the contents. +// +// The loader runs before slog is initialized in cmd/minstrel/main.go; +// log.Printf is the project's pre-logger convention. +func resolveStreamSecret(cfg *Config) error { + if raw := strings.TrimSpace(os.Getenv("MINSTREL_STREAM_SECRET")); raw != "" { + decoded, err := decodeBase64Secret(raw) + if err != nil { + return fmt.Errorf("decode MINSTREL_STREAM_SECRET: %w", err) + } + cfg.StreamSecret = decoded + return nil + } + if cfg.Storage.DataDir == "" { + return fmt.Errorf("stream secret: empty storage.data_dir; " + + "set MINSTREL_STREAM_SECRET or storage.data_dir") + } + path := filepath.Join(cfg.Storage.DataDir, streamSecretFile) + if buf, err := os.ReadFile(path); err == nil && len(buf) > 0 { + decoded, err := decodeBase64Secret(strings.TrimSpace(string(buf))) + if err != nil { + return fmt.Errorf("decode %s: %w", path, err) + } + cfg.StreamSecret = decoded + return nil + } + raw := make([]byte, streamSecretBytes) + if _, err := rand.Read(raw); err != nil { + return fmt.Errorf("auto-gen stream secret: %w", err) + } + encoded := base64.RawURLEncoding.EncodeToString(raw) + if err := os.MkdirAll(cfg.Storage.DataDir, 0o755); err != nil { + return fmt.Errorf("ensure data_dir for stream secret: %w", err) + } + if err := os.WriteFile(path, []byte(encoded), 0o600); err != nil { + return fmt.Errorf("persist stream secret: %w", err) + } + log.Printf("config: auto-generated MINSTREL_STREAM_SECRET (persisted to %s)", path) + cfg.StreamSecret = raw + return nil +} + +// decodeBase64Secret accepts base64-url with OR without padding; the +// stream-secret bytes are opaque so callers shouldn't have to know which +// encoder produced their string. Returns an error on any other shape. +func decodeBase64Secret(s string) ([]byte, error) { + if buf, err := base64.RawURLEncoding.DecodeString(s); err == nil { + return buf, nil + } + return base64.URLEncoding.DecodeString(s) +} + func applyEnv(cfg *Config) { if v, ok := os.LookupEnv("MINSTREL_SERVER_ADDRESS"); ok { cfg.Server.Address = v diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 7768f3c9..1969bb67 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1,11 +1,23 @@ package config import ( + "encoding/base64" "os" "path/filepath" + "strings" "testing" ) +// stubStreamSecret keeps existing tests focused on the fields they +// assert against by short-circuiting the auto-generation path (which +// would otherwise write ./data/stream_secret in the test workspace). +// New tests that exercise the resolver directly do NOT use this. +func stubStreamSecret(t *testing.T) { + t.Helper() + t.Setenv("MINSTREL_STREAM_SECRET", + base64.RawURLEncoding.EncodeToString([]byte("test-stream-secret-stub-1234567890"))) +} + func TestDefault(t *testing.T) { cfg := Default() if cfg.Server.Address != ":4533" { @@ -17,6 +29,7 @@ func TestDefault(t *testing.T) { } func TestLoadYAML(t *testing.T) { + stubStreamSecret(t) dir := t.TempDir() path := filepath.Join(dir, "config.yaml") content := []byte(`server: @@ -46,6 +59,7 @@ log: } func TestLoadMissingFileReturnsDefaults(t *testing.T) { + stubStreamSecret(t) cfg, err := Load(filepath.Join(t.TempDir(), "does-not-exist.yaml")) if err != nil { t.Fatalf("Load: %v", err) @@ -56,6 +70,7 @@ func TestLoadMissingFileReturnsDefaults(t *testing.T) { } func TestEnvOverrides(t *testing.T) { + stubStreamSecret(t) t.Setenv("MINSTREL_SERVER_ADDRESS", ":8080") t.Setenv("MINSTREL_DATABASE_URL", "postgres://env") t.Setenv("MINSTREL_LOG_LEVEL", "WARN") @@ -80,6 +95,7 @@ func TestEnvOverrides(t *testing.T) { } func TestLibraryEnvOverrides(t *testing.T) { + stubStreamSecret(t) t.Setenv("MINSTREL_LIBRARY_SCAN_PATHS", "/music:/other::/third") t.Setenv("MINSTREL_LIBRARY_SCAN_ON_STARTUP", "true") @@ -102,6 +118,7 @@ func TestLibraryEnvOverrides(t *testing.T) { } func TestSubsonicEnvOverride(t *testing.T) { + stubStreamSecret(t) t.Setenv("MINSTREL_SUBSONIC_ALLOW_PLAINTEXT_PASSWORD", "true") cfg, err := Load("") if err != nil { @@ -113,6 +130,7 @@ func TestSubsonicEnvOverride(t *testing.T) { } func TestLibraryYAMLLoads(t *testing.T) { + stubStreamSecret(t) dir := t.TempDir() path := filepath.Join(dir, "config.yaml") content := []byte(`library: @@ -147,6 +165,7 @@ func TestBrandingDefaults(t *testing.T) { } func TestBrandingYAMLOverride(t *testing.T) { + stubStreamSecret(t) dir := t.TempDir() path := filepath.Join(dir, "config.yaml") content := []byte(`branding: @@ -168,7 +187,81 @@ func TestBrandingYAMLOverride(t *testing.T) { } } +func TestStreamSecret_EnvOverride(t *testing.T) { + want := []byte("hello-stream-secret-from-env-32b") + t.Setenv("MINSTREL_STREAM_SECRET", base64.RawURLEncoding.EncodeToString(want)) + t.Setenv("MINSTREL_STORAGE_DATA_DIR", t.TempDir()) + cfg, err := Load("") + if err != nil { + t.Fatalf("Load: %v", err) + } + if string(cfg.StreamSecret) != string(want) { + t.Fatalf("StreamSecret mismatch: got %q want %q", cfg.StreamSecret, want) + } +} + +func TestStreamSecret_AutoGenPersistsToDataDir(t *testing.T) { + _ = os.Unsetenv("MINSTREL_STREAM_SECRET") + dataDir := t.TempDir() + t.Setenv("MINSTREL_STORAGE_DATA_DIR", dataDir) + + cfg, err := Load("") + if err != nil { + t.Fatalf("Load: %v", err) + } + if len(cfg.StreamSecret) != streamSecretBytes { + t.Fatalf("StreamSecret len = %d, want %d", len(cfg.StreamSecret), streamSecretBytes) + } + path := filepath.Join(dataDir, streamSecretFile) + buf, err := os.ReadFile(path) + if err != nil { + t.Fatalf("expected persisted secret at %s: %v", path, err) + } + decoded, err := base64.RawURLEncoding.DecodeString(strings.TrimSpace(string(buf))) + if err != nil { + t.Fatalf("persisted secret is not raw-url-base64: %v", err) + } + if string(decoded) != string(cfg.StreamSecret) { + t.Fatal("persisted file does not match returned secret") + } + info, err := os.Stat(path) + if err != nil { + t.Fatalf("stat: %v", err) + } + // 0600 — never let other users read the HMAC key. + if perm := info.Mode().Perm(); perm != 0o600 { + t.Fatalf("perm = %o, want 0600", perm) + } +} + +func TestStreamSecret_LoadsPersistedOnSecondBoot(t *testing.T) { + _ = os.Unsetenv("MINSTREL_STREAM_SECRET") + dataDir := t.TempDir() + t.Setenv("MINSTREL_STORAGE_DATA_DIR", dataDir) + + first, err := Load("") + if err != nil { + t.Fatalf("Load #1: %v", err) + } + second, err := Load("") + if err != nil { + t.Fatalf("Load #2: %v", err) + } + if string(first.StreamSecret) != string(second.StreamSecret) { + t.Fatal("second Load got a different secret — file fallback didn't fire") + } +} + +func TestStreamSecret_RejectsMalformedEnvValue(t *testing.T) { + t.Setenv("MINSTREL_STREAM_SECRET", "not!base64!!!") + t.Setenv("MINSTREL_STORAGE_DATA_DIR", t.TempDir()) + if _, err := Load(""); err == nil { + t.Fatal("expected error on malformed MINSTREL_STREAM_SECRET") + } +} + func TestBrandingEnvOverride(t *testing.T) { + stubStreamSecret(t) t.Setenv("MINSTREL_BRANDING_APP_NAME", "Office Music") t.Setenv("MINSTREL_BRANDING_DESCRIPTION", "Office tunes.") cfg, err := Load("") diff --git a/internal/playlists/system.go b/internal/playlists/system.go index fc55eed6..b2eeead5 100644 --- a/internal/playlists/system.go +++ b/internal/playlists/system.go @@ -278,16 +278,23 @@ func RefreshableSystemKind(key string) bool { // here (plus its candidate query). Order is the materialize order; // it has no functional effect (atomic replace + per-playlist // collage are order-independent). -var systemPlaylistRegistry = []systemPlaylistKind{ - {Key: "for_you", Singleton: true, Produce: produceForYou}, - {Key: "songs_like_artist", Singleton: false, Produce: produceSeedMixes}, - {Key: "discover", Singleton: true, Produce: produceDiscover}, - {Key: "deep_cuts", Singleton: true, Produce: produceDeepCuts}, - {Key: "rediscover", Singleton: true, Produce: produceRediscover}, - {Key: "new_for_you", Singleton: true, Produce: produceNewForYou}, - {Key: "on_this_day", Singleton: true, Produce: produceOnThisDay}, - {Key: "first_listens", Singleton: true, Produce: produceFirstListens}, -} +var systemPlaylistRegistry = func() []systemPlaylistKind { + out := []systemPlaylistKind{ + {Key: "for_you", Singleton: true, Produce: produceForYou}, + {Key: "songs_like_artist", Singleton: false, Produce: produceSeedMixes}, + {Key: "discover", Singleton: true, Produce: produceDiscover}, + } + // The five discovery mixes share one Produce closure factory keyed + // by a per-mix spec; spec list + factory live in system_mixes.go. + for _, spec := range discoveryMixSpecs { + out = append(out, systemPlaylistKind{ + Key: spec.variant, + Singleton: true, + Produce: produceDiscoveryMix(spec), + }) + } + return out +}() // systemForYouSourceLimits is a deeper candidate pool than the radio // default. On a self-hosted library without ListenBrainz similarity diff --git a/internal/playlists/system_mixes.go b/internal/playlists/system_mixes.go index e57233ea..4bd862e4 100644 --- a/internal/playlists/system_mixes.go +++ b/internal/playlists/system_mixes.go @@ -3,6 +3,7 @@ package playlists import ( "context" "log/slog" + "math/rand" "time" "github.com/jackc/pgx/v5/pgtype" @@ -10,29 +11,206 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" ) -// Discovery mixes (#419-423). Each is one candidate query + a thin -// producer registered in systemPlaylistRegistry. They follow the -// Discover model: a SQL query returns ordered (id, album_id, -// artist_id) rows; we optionally diversity-cap, truncate, and emit -// rankedCandidate (Score unused — SQL gave the ranking). All are -// singleton kinds, so the generic by-kind refresh/shuffle endpoints -// and per-tile refresh affordance work with zero client changes. +// Discovery mixes (#419-423). One generic producer + per-mix spec. +// Replaces the five near-identical produceXxx functions that all +// fetched ranked (id, album_id, artist_id) rows, diversified, +// truncated, and emitted a single playlist. +// +// Day-keying is a per-mix property captured by `dailyRotate`: +// +// - DeepCuts / OnThisDay — SQL already day-keys via +// ORDER BY md5(t.id::text || $2::text), so the Go producer keeps +// SQL order. `dailyRotate: false`. +// +// - Rediscover / NewForYou / FirstListens — SQL accepts only $1 +// user_id and produces deterministic ordering. `dailyRotate: +// true` applies a daily-deterministic rotate-left of the pool +// BEFORE diversify+truncate so each day's top-100 surfaces a +// different slice while contiguous-block ordering within each +// slice is preserved (matters for FirstListens / NewForYou which +// are album-coherent — rotation walks the album boundary cleanly +// rather than scrambling within an album). +// +// Diversity is `true` for every mix: per-album <= 2 / per-artist <= 3. +// On thin libraries where the cap would chop the pool below 100, +// finishMix tops up from the uncapped raw pool so the mix still +// ships a full-length playlist — see topUpFromRaw. // discoveryMixLen caps each mix at the same depth as For-You / // Discover so shuffle-on-play has a varied pool within a day. const discoveryMixLen = 100 -// finishMix caps (per-album<=2 / per-artist<=3) when diversify is -// set, truncates to discoveryMixLen, and converts to the insert -// type. Album-coherent mixes (New for you, First Listens) pass -// diversify=false so whole albums survive. -func finishMix(rows []discoverTrack, diversify bool) []rankedCandidate { - pool := rows - if diversify { - pool = capByAlbumAndArtist(pool) +// discoveryMixSpec describes one discovery mix. The unified producer +// reads the spec and runs a single code path for all variants. +type discoveryMixSpec struct { + name string + variant string + diversify bool + + // dailyRotate, when true, applies a daily-deterministic offset + // rotation to the candidate pool BEFORE diversify+truncate so the + // top discoveryMixLen rotates day-over-day. Set on variants whose + // SQL ORDER is invariant to dateStr (Rediscover, FirstListens). + // Leave false when the SQL already day-keys (DeepCuts, OnThisDay) + // or when day-over-day stability is the intended UX (NewForYou). + dailyRotate bool + + // fetch returns the raw ranked rows. dateStr is supplied for + // queries that accept it (passed as the second positional arg + // historically); queries that don't accept it ignore the param. + fetch func(context.Context, *dbq.Queries, pgtype.UUID, string) ([]discoverTrack, error) +} + +// produceDiscoveryMix returns a systemPlaylistKind.Produce closure +// bound to the given spec. Registered in systemPlaylistRegistry; see +// the discoveryMixSpecs slice below for the concrete instances. +func produceDiscoveryMix(spec discoveryMixSpec) systemPlaylistProducer { + return func( + ctx context.Context, q *dbq.Queries, logger *slog.Logger, + userID pgtype.UUID, dateStr string, _ time.Time, + ) ([]builtPlaylist, error) { + rows, err := spec.fetch(ctx, q, userID, dateStr) + if err != nil { + logger.Warn("system playlist: "+spec.variant+" query failed; skipping", + "user_id", uuidStringPL(userID), "err", err) + return nil, nil + } + pool := rows + if spec.dailyRotate { + pool = rotateForDay(pool, userID, dateStr) + } + return emit(spec.name, spec.variant, finishMix(pool, spec.diversify)), nil } - if len(pool) > discoveryMixLen { - pool = pool[:discoveryMixLen] +} + +// rotateForDay rotates pool left by a daily-deterministic offset so +// each day's downstream truncate-to-N surfaces a different slice of +// the pool while contiguous-block ordering inside the slice is +// preserved. Empty / single-element pools pass through unchanged. +func rotateForDay(pool []discoverTrack, userID pgtype.UUID, dateStr string) []discoverTrack { + n := len(pool) + if n <= 1 { + return pool + } + rng := rand.New(rand.NewSource(int64(userIDHash(userID, dateStr)))) + offset := rng.Intn(n) + rotated := make([]discoverTrack, 0, n) + rotated = append(rotated, pool[offset:]...) + rotated = append(rotated, pool[:offset]...) + return rotated +} + +// discoveryMixSpecs is the concrete spec list used by the registry in +// system.go. Adding a new mix = one entry here + the candidate query. +// +// Order has no functional effect (insert-time atomic replace is order +// independent); listed in the same order as the historical registry. +var discoveryMixSpecs = []discoveryMixSpec{ + { + name: "Deep Cuts", variant: "deep_cuts", + diversify: true, dailyRotate: false, // SQL day-keys via md5(id||$2) + fetch: func(ctx context.Context, q *dbq.Queries, uid pgtype.UUID, ds string) ([]discoverTrack, error) { + rows, err := q.ListDeepCutsTracks(ctx, dbq.ListDeepCutsTracksParams{ + UserID: uid, Column2: ds, + }) + if err != nil { + return nil, err + } + out := make([]discoverTrack, len(rows)) + for i, r := range rows { + out[i] = discoverTrack{ID: r.ID, AlbumID: r.AlbumID, ArtistID: r.ArtistID} + } + return out, nil + }, + }, + { + name: "Rediscover", variant: "rediscover", + diversify: true, dailyRotate: true, // SQL has no date arg + fetch: func(ctx context.Context, q *dbq.Queries, uid pgtype.UUID, _ string) ([]discoverTrack, error) { + rows, err := q.ListRediscoverTracks(ctx, uid) + if err != nil { + return nil, err + } + out := make([]discoverTrack, len(rows)) + for i, r := range rows { + out[i] = discoverTrack{ID: r.ID, AlbumID: r.AlbumID, ArtistID: r.ArtistID} + } + return out, nil + }, + }, + { + name: "New for you", variant: "new_for_you", + diversify: true, dailyRotate: true, // operator wants daily rotation on all deterministic mixes + fetch: func(ctx context.Context, q *dbq.Queries, uid pgtype.UUID, _ string) ([]discoverTrack, error) { + rows, err := q.ListNewForYouTracks(ctx, uid) + if err != nil { + return nil, err + } + out := make([]discoverTrack, len(rows)) + for i, r := range rows { + out[i] = discoverTrack{ID: r.ID, AlbumID: r.AlbumID, ArtistID: r.ArtistID} + } + return out, nil + }, + }, + { + name: "On this day", variant: "on_this_day", + diversify: true, dailyRotate: false, // SQL day-keys via md5(id||$2) + fetch: func(ctx context.Context, q *dbq.Queries, uid pgtype.UUID, ds string) ([]discoverTrack, error) { + rows, err := q.ListOnThisDayTracks(ctx, dbq.ListOnThisDayTracksParams{ + UserID: uid, Column2: ds, + }) + if err != nil { + return nil, err + } + out := make([]discoverTrack, len(rows)) + for i, r := range rows { + out[i] = discoverTrack{ID: r.ID, AlbumID: r.AlbumID, ArtistID: r.ArtistID} + } + return out, nil + }, + }, + { + name: "First listens", variant: "first_listens", + diversify: true, dailyRotate: true, // SQL has no date arg; daily rotate + diversity top-up + fetch: func(ctx context.Context, q *dbq.Queries, uid pgtype.UUID, _ string) ([]discoverTrack, error) { + rows, err := q.ListFirstListensTracks(ctx, uid) + if err != nil { + return nil, err + } + out := make([]discoverTrack, len(rows)) + for i, r := range rows { + out[i] = discoverTrack{ID: r.ID, AlbumID: r.AlbumID, ArtistID: r.ArtistID} + } + return out, nil + }, + }, +} + +// finishMix applies diversity caps (per-album <= 2 / per-artist <= 3) +// when diversify is set, with a top-up fallback when caps strip the +// pool below discoveryMixLen: the capped result is filled out with +// non-capped tracks (preserving original SQL order) until the target +// is hit or the raw pool runs out. +// +// The fallback matters on small / album-heavy libraries — the cap +// can chop a 200-row pool down to 40, and we'd rather ship a partly- +// diversified 100 than a strictly-diversified 40. On rich libraries +// the cap yields >= 100 and the top-up path never runs. +func finishMix(rows []discoverTrack, diversify bool) []rankedCandidate { + var pool []discoverTrack + if diversify { + capped := capByAlbumAndArtist(rows) + if len(capped) >= discoveryMixLen { + pool = capped[:discoveryMixLen] + } else { + pool = topUpFromRaw(capped, rows, discoveryMixLen) + } + } else { + pool = rows + if len(pool) > discoveryMixLen { + pool = pool[:discoveryMixLen] + } } if len(pool) == 0 { return nil @@ -44,6 +222,32 @@ func finishMix(rows []discoverTrack, diversify bool) []rankedCandidate { return tracks } +// topUpFromRaw appends non-capped tracks from raw (in their original +// order) onto capped, skipping any already present, until the result +// reaches target or raw is exhausted. Preserves SQL ranking semantics +// for the non-diverse fill so the topped-up tail still trends best- +// first within each album. +func topUpFromRaw(capped, raw []discoverTrack, target int) []discoverTrack { + if len(capped) >= target { + return capped[:target] + } + seen := make(map[pgtype.UUID]struct{}, len(capped)) + for _, t := range capped { + seen[t.ID] = struct{}{} + } + pool := capped + for _, t := range raw { + if _, in := seen[t.ID]; in { + continue + } + pool = append(pool, t) + if len(pool) >= target { + break + } + } + return pool +} + // emit wraps the finished track list in a single builtPlaylist (the // discovery mixes are all singletons). nil tracks → no playlist. func emit(name, variant string, tracks []rankedCandidate) []builtPlaylist { @@ -52,94 +256,3 @@ func emit(name, variant string, tracks []rankedCandidate) []builtPlaylist { } return []builtPlaylist{{Name: name, Variant: variant, Tracks: tracks}} } - -func produceDeepCuts( - ctx context.Context, q *dbq.Queries, logger *slog.Logger, - userID pgtype.UUID, dateStr string, _ time.Time, -) ([]builtPlaylist, error) { - rows, err := q.ListDeepCutsTracks(ctx, dbq.ListDeepCutsTracksParams{ - UserID: userID, Column2: dateStr, - }) - if err != nil { - logger.Warn("system playlist: deep-cuts query failed; skipping", - "user_id", uuidStringPL(userID), "err", err) - return nil, nil - } - dt := make([]discoverTrack, len(rows)) - for i, r := range rows { - dt[i] = discoverTrack{ID: r.ID, AlbumID: r.AlbumID, ArtistID: r.ArtistID} - } - return emit("Deep Cuts", "deep_cuts", finishMix(dt, true)), nil -} - -func produceRediscover( - ctx context.Context, q *dbq.Queries, logger *slog.Logger, - userID pgtype.UUID, _ string, _ time.Time, -) ([]builtPlaylist, error) { - rows, err := q.ListRediscoverTracks(ctx, userID) - if err != nil { - logger.Warn("system playlist: rediscover query failed; skipping", - "user_id", uuidStringPL(userID), "err", err) - return nil, nil - } - dt := make([]discoverTrack, len(rows)) - for i, r := range rows { - dt[i] = discoverTrack{ID: r.ID, AlbumID: r.AlbumID, ArtistID: r.ArtistID} - } - return emit("Rediscover", "rediscover", finishMix(dt, true)), nil -} - -func produceNewForYou( - ctx context.Context, q *dbq.Queries, logger *slog.Logger, - userID pgtype.UUID, _ string, _ time.Time, -) ([]builtPlaylist, error) { - rows, err := q.ListNewForYouTracks(ctx, userID) - if err != nil { - logger.Warn("system playlist: new-for-you query failed; skipping", - "user_id", uuidStringPL(userID), "err", err) - return nil, nil - } - dt := make([]discoverTrack, len(rows)) - for i, r := range rows { - dt[i] = discoverTrack{ID: r.ID, AlbumID: r.AlbumID, ArtistID: r.ArtistID} - } - // Album-coherent: no diversity cap so whole new albums survive. - return emit("New for you", "new_for_you", finishMix(dt, false)), nil -} - -func produceOnThisDay( - ctx context.Context, q *dbq.Queries, logger *slog.Logger, - userID pgtype.UUID, dateStr string, _ time.Time, -) ([]builtPlaylist, error) { - rows, err := q.ListOnThisDayTracks(ctx, dbq.ListOnThisDayTracksParams{ - UserID: userID, Column2: dateStr, - }) - if err != nil { - logger.Warn("system playlist: on-this-day query failed; skipping", - "user_id", uuidStringPL(userID), "err", err) - return nil, nil - } - dt := make([]discoverTrack, len(rows)) - for i, r := range rows { - dt[i] = discoverTrack{ID: r.ID, AlbumID: r.AlbumID, ArtistID: r.ArtistID} - } - return emit("On this day", "on_this_day", finishMix(dt, true)), nil -} - -func produceFirstListens( - ctx context.Context, q *dbq.Queries, logger *slog.Logger, - userID pgtype.UUID, _ string, _ time.Time, -) ([]builtPlaylist, error) { - rows, err := q.ListFirstListensTracks(ctx, userID) - if err != nil { - logger.Warn("system playlist: first-listens query failed; skipping", - "user_id", uuidStringPL(userID), "err", err) - return nil, nil - } - dt := make([]discoverTrack, len(rows)) - for i, r := range rows { - dt[i] = discoverTrack{ID: r.ID, AlbumID: r.AlbumID, ArtistID: r.ArtistID} - } - // Album-coherent (tiered by liked/played artist in SQL): no cap. - return emit("First listens", "first_listens", finishMix(dt, false)), nil -} diff --git a/internal/server/server.go b/internal/server/server.go index cf381871..b402d2af 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -89,6 +89,12 @@ type Server struct { // PUT /api/me/timezone and POST /api/auth/register can call // Refresh synchronously. PlaylistScheduler *playlists.Scheduler + // StreamSecret is the HMAC key used by /api/cast/stream-token to + // mint signed UPnP / Sonos stream URLs and by /api/tracks/{id}/stream + // to verify them. Sourced from config.Config.StreamSecret. Tests that + // leave it nil leave the cookie path intact and reject all signed + // tokens (HMAC of empty key matches nothing a client could mint). + StreamSecret []byte } func New(logger *slog.Logger, pool *pgxpool.Pool, scanner ScanTrigger, subCfg subsonic.Config, eventsCfg config.EventsConfig, recCfg config.RecommendationConfig, dataDir string, brandingCfg config.BrandingConfig, coverEnricher *coverart.Enricher, coverSettings *coverart.SettingsService, libraryScanner *library.Scanner, scanCfg library.RunScanConfig, scheduler *library.Scheduler) *Server { @@ -138,7 +144,7 @@ func (s *Server) Router() http.Handler { if bus == nil { bus = eventbus.New() } - api.Mount(r, s.Pool, s.Logger, writer, s.RecommendationCfg, lidarrCfg, lidarrReqs, lidarrQuar, tracksSvc, playlistsSvc, s.CoverEnricher, s.CoverSettings, s.LibraryScanner, s.ScanCfg, s.Scheduler, s.DataDir, smtpSender, bus, s.PlaylistScheduler) + api.Mount(r, s.Pool, s.Logger, writer, s.RecommendationCfg, lidarrCfg, lidarrReqs, lidarrQuar, tracksSvc, playlistsSvc, s.CoverEnricher, s.CoverSettings, s.LibraryScanner, s.ScanCfg, s.Scheduler, s.DataDir, smtpSender, bus, s.PlaylistScheduler, s.StreamSecret) // /api/admin/scan is the only admin route owned by the server package // (it needs the Scanner). Register it as a single inline-middleware // route — using r.Route("/api/admin", ...) here would create a second