Merge pull request 'Discover artwork + Library icon + notification tap routing' (#73) from dev into main

android/app/build.gradle.kts

@@ -139,6 +139,7 @@ dependencies {
     implementation(libs.compose.ui)
     implementation(libs.compose.ui.graphics)
     implementation(libs.compose.material3)
+    implementation(libs.compose.material.icons.extended)
     implementation(libs.compose.ui.text.google.fonts)
     debugImplementation(libs.compose.ui.tooling)
     implementation(libs.compose.ui.tooling.preview)

android/app/src/main/java/com/fabledsword/minstrel/MainActivity.kt

@@ -1,5 +1,6 @@
 package com.fabledsword.minstrel
 
+import android.content.Intent
 import android.os.Bundle
 import androidx.activity.ComponentActivity
 import androidx.activity.compose.setContent
@@ -11,6 +12,7 @@ import androidx.compose.material3.MaterialTheme
 import androidx.compose.material3.Surface
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.CompositionLocalProvider
+import androidx.compose.runtime.LaunchedEffect
 import androidx.compose.runtime.getValue
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
@@ -22,10 +24,14 @@ import com.fabledsword.minstrel.cache.CachedTrackIds
 import com.fabledsword.minstrel.nav.DetailSeedCache
 import com.fabledsword.minstrel.nav.LocalDetailSeedCache
 import com.fabledsword.minstrel.nav.MinstrelNavGraph
+import com.fabledsword.minstrel.nav.NowPlaying
 import com.fabledsword.minstrel.shared.widgets.LocalCachedTrackIds
 import com.fabledsword.minstrel.theme.MinstrelTheme
 import com.fabledsword.minstrel.theme.ThemePreferenceViewModel
 import dagger.hilt.android.AndroidEntryPoint
+import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.flow.StateFlow
+import kotlinx.coroutines.flow.asStateFlow
 import javax.inject.Inject
 
 @AndroidEntryPoint
@@ -33,10 +39,46 @@ class MainActivity : ComponentActivity() {
     @Inject lateinit var seedCache: DetailSeedCache
     @Inject lateinit var cachedTrackIds: CachedTrackIds
 
+    // Flipped to true when the user taps the media notification (or
+    // any other entry point that asks for the full player). The App
+    // composable observes this, navigates to NowPlaying once the
+    // NavHost is ready, then calls back to reset the flag so the
+    // navigation doesn't re-fire on the next recomposition.
+    private val pendingOpenNowPlaying = MutableStateFlow(false)
+
     override fun onCreate(savedInstanceState: Bundle?) {
         super.onCreate(savedInstanceState)
         enableEdgeToEdge()
-        setContent { App(seedCache = seedCache, cachedTrackIds = cachedTrackIds) }
+        consumeOpenNowPlayingIntent(intent)
+        setContent {
+            App(
+                seedCache = seedCache,
+                cachedTrackIds = cachedTrackIds,
+                pendingOpenNowPlaying = pendingOpenNowPlaying.asStateFlow(),
+                onOpenedNowPlaying = { pendingOpenNowPlaying.value = false },
+            )
+        }
+    }
+
+    override fun onNewIntent(intent: Intent) {
+        super.onNewIntent(intent)
+        consumeOpenNowPlayingIntent(intent)
+    }
+
+    private fun consumeOpenNowPlayingIntent(intent: Intent?) {
+        if (intent?.getBooleanExtra(EXTRA_OPEN_NOW_PLAYING, false) == true) {
+            pendingOpenNowPlaying.value = true
+            // Strip the extra so a subsequent config-change recreation
+            // doesn't re-trigger the navigation.
+            intent.removeExtra(EXTRA_OPEN_NOW_PLAYING)
+        }
+    }
+
+    companion object {
+        /** PendingIntent extra set by [com.fabledsword.minstrel.player.MinstrelPlayerService]
+         *  so a media-notification tap lands on the full NowPlaying screen
+         *  instead of whatever shell route MainActivity last rendered. */
+        const val EXTRA_OPEN_NOW_PLAYING = "com.fabledsword.minstrel.action.OPEN_NOW_PLAYING"
     }
 }
 
@@ -44,11 +86,14 @@ class MainActivity : ComponentActivity() {
 private fun App(
     seedCache: DetailSeedCache,
     cachedTrackIds: CachedTrackIds,
+    pendingOpenNowPlaying: StateFlow<Boolean>,
+    onOpenedNowPlaying: () -> Unit,
     themeVm: ThemePreferenceViewModel = hiltViewModel(),
     gate: AuthGateViewModel = hiltViewModel(),
 ) {
     val theme by themeVm.themeMode.collectAsStateWithLifecycle()
     val cached by cachedTrackIds.ids.collectAsStateWithLifecycle()
+    val pending by pendingOpenNowPlaying.collectAsStateWithLifecycle()
     MinstrelTheme(darkOverride = theme.toDarkOverride()) {
         CompositionLocalProvider(
             LocalDetailSeedCache provides seedCache,
@@ -65,6 +110,18 @@ private fun App(
                 // `ShellScaffold` wrap; full-screen routes (NowPlaying /
                 // Queue / unauthenticated) bypass the shell entirely.
                 val navController = rememberNavController()
+                // Honour a pending notification-tap once the NavHost is
+                // mounted. launchSingleTop avoids stacking copies of
+                // NowPlaying if the user taps the notification while
+                // already on it; the callback clears the flag so a later
+                // recomposition (config change, theme switch) doesn't
+                // re-navigate.
+                LaunchedEffect(pending, navController) {
+                    if (pending) {
+                        navController.navigate(NowPlaying) { launchSingleTop = true }
+                        onOpenedNowPlaying()
+                    }
+                }
                 MinstrelNavGraph(
                     navController = navController,
                     startDestination = resolved,

android/app/src/main/java/com/fabledsword/minstrel/api/BaseUrlInterceptor.kt

@@ -9,16 +9,23 @@ import javax.inject.Inject
 import javax.inject.Singleton
 
 /**
- * Rewrites every outgoing request's scheme/host/port to match the
+ * Rewrites Minstrel-server requests' scheme/host/port to match the
  * current [AuthStore.baseUrl]. Retrofit's `.baseUrl(...)` is read
  * once at Retrofit creation, but AuthStore.baseUrl loads from Room
  * asynchronously — so at injection time the Retrofit instance is
  * frozen pointing at the [AuthStore.DEFAULT_BASE_URL] placeholder.
  *
  * This interceptor closes the gap: Retrofit can stay built with the
- * placeholder forever, and every actual request gets retargeted at
+ * placeholder forever, and every actual Minstrel-bound request gets
- * the live AuthStore value. Lets the user change server URL in
+ * retargeted at the live AuthStore value. Lets the user change
- * Settings without an app relaunch (Phase 11 wiring).
+ * server URL in Settings without an app relaunch (Phase 11 wiring).
+ *
+ * Only requests whose host is the [PLACEHOLDER_HOST] sentinel are
+ * rewritten. Absolute external URLs — Lidarr-surfaced artwork from
+ * artwork.musicbrainz.org / coverartarchive.org, for example — must
+ * reach their authored host unchanged; rewriting them to the
+ * Minstrel host produced 404s the user saw as missing Discover
+ * suggestion covers.
  *
  * No-op when the stored base URL is unparseable (falls through to
  * whatever Retrofit had) — that case shows up as a transport-level
@@ -31,13 +38,26 @@ class BaseUrlInterceptor @Inject constructor(
 
     override fun intercept(chain: Interceptor.Chain): Response {
         val original = chain.request()
-        val baseUrl = authStore.baseUrl.value.toHttpUrlOrNull()
+        // Early-bail keeps the no-op path for external URLs cheap
-            ?: return chain.proceed(original)
+        // (no AuthStore read, no URL parse).
-        val rewritten: HttpUrl = original.url.newBuilder()
+        if (original.url.host != PLACEHOLDER_HOST) return chain.proceed(original)
-            .scheme(baseUrl.scheme)
+        // Unparseable baseUrl folds into the same proceed path — keeps
-            .host(baseUrl.host)
+        // detekt's ReturnCount happy by avoiding a second early return.
-            .port(baseUrl.port)
+        val rewritten: HttpUrl = authStore.baseUrl.value.toHttpUrlOrNull()?.let { baseUrl ->
-            .build()
+            original.url.newBuilder()
+                .scheme(baseUrl.scheme)
+                .host(baseUrl.host)
+                .port(baseUrl.port)
+                .build()
+        } ?: original.url
         return chain.proceed(original.newBuilder().url(rewritten).build())
     }
+
+    companion object {
+        /** Sentinel host used by Retrofit + every code path that builds
+         *  a Minstrel-server URL via the `http://placeholder.invalid/...`
+         *  form (see [com.fabledsword.minstrel.shared.resolveServerUrl],
+         *  CoverUrls.kt, AlbumRef/TrackRef cover getters). */
+        const val PLACEHOLDER_HOST = "placeholder.invalid"
+    }
 }

android/app/src/main/java/com/fabledsword/minstrel/player/MinstrelPlayerService.kt

@@ -1,9 +1,11 @@
 package com.fabledsword.minstrel.player
 
+import android.app.PendingIntent
 import android.content.Intent
 import androidx.media3.common.Player
 import androidx.media3.session.MediaSession
 import androidx.media3.session.MediaSessionService
+import com.fabledsword.minstrel.MainActivity
 import dagger.hilt.android.AndroidEntryPoint
 import javax.inject.Inject
 
@@ -38,7 +40,36 @@ class MinstrelPlayerService : MediaSessionService() {
     override fun onCreate() {
         super.onCreate()
         val player = playerFactory.build()
-        mediaSession = MediaSession.Builder(this, player).build()
+        mediaSession = MediaSession.Builder(this, player)
+            .setSessionActivity(buildNowPlayingPendingIntent())
+            .build()
+    }
+
+    /**
+     * Intent that the media notification / lock-screen card / Pixel
+     * Watch tile launches when tapped. Carries an extra MainActivity
+     * consumes to navigate the NavController straight to the full
+     * NowPlaying screen — without it, Media3 defaults to the launcher
+     * activity entry point, which lands on whatever shell route
+     * MainActivity last rendered.
+     *
+     * FLAG_ACTIVITY_SINGLE_TOP keeps the existing MainActivity instance
+     * alive when one is already foregrounded; onNewIntent fires and
+     * picks up the fresh extra. FLAG_IMMUTABLE is required on
+     * Android 12+; FLAG_UPDATE_CURRENT keeps the extra in sync if the
+     * Builder rebuilds the PendingIntent.
+     */
+    private fun buildNowPlayingPendingIntent(): PendingIntent {
+        val intent = Intent(this, MainActivity::class.java).apply {
+            flags = Intent.FLAG_ACTIVITY_SINGLE_TOP
+            putExtra(MainActivity.EXTRA_OPEN_NOW_PLAYING, true)
+        }
+        return PendingIntent.getActivity(
+            this,
+            REQUEST_OPEN_NOW_PLAYING,
+            intent,
+            PendingIntent.FLAG_IMMUTABLE or PendingIntent.FLAG_UPDATE_CURRENT,
+        )
     }
 
     override fun onGetSession(controllerInfo: MediaSession.ControllerInfo): MediaSession? =
@@ -61,4 +92,11 @@ class MinstrelPlayerService : MediaSessionService() {
         mediaSession = null
         super.onDestroy()
     }
+
+    private companion object {
+        // Stable request code for the session-activity PendingIntent.
+        // Arbitrary but distinct from any other PendingIntent we ever
+        // build against MainActivity.
+        const val REQUEST_OPEN_NOW_PLAYING = 0x100
+    }
 }

android/app/src/main/java/com/fabledsword/minstrel/shared/widgets/MainAppBarActions.kt

@@ -11,11 +11,12 @@ import androidx.compose.runtime.getValue
 import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.remember
 import androidx.compose.runtime.setValue
+import androidx.compose.material.icons.Icons
+import androidx.compose.material.icons.outlined.LibraryMusic
 import androidx.hilt.navigation.compose.hiltViewModel
 import androidx.lifecycle.compose.collectAsStateWithLifecycle
 import androidx.navigation.NavHostController
 import com.composables.icons.lucide.House
-import com.composables.icons.lucide.LibraryBig
 import com.composables.icons.lucide.Lucide
 import com.composables.icons.lucide.EllipsisVertical
 import com.composables.icons.lucide.Search as SearchIcon
@@ -58,7 +59,14 @@ fun MainAppBarActions(
         }
         if (currentRouteName != Library::class.qualifiedName) {
             IconButton(onClick = { navController.navigate(Library) }) {
-                Icon(Lucide.LibraryBig, contentDescription = "Library")
+                // Intentional cross-family icon — Lucide has no
+                // equivalent "library + music" glyph and the literal
+                // library icons (Library / LibraryBig / SquareLibrary)
+                // don't read as "music collection" without a label.
+                // Material's Outlined LibraryMusic is the canonical
+                // music-library symbol; the outlined variant matches
+                // Lucide's stroke style closely enough to blend.
+                Icon(Icons.Outlined.LibraryMusic, contentDescription = "Library")
             }
         }
         if (currentRouteName != SearchRoute::class.qualifiedName) {

android/app/src/test/java/com/fabledsword/minstrel/api/BaseUrlInterceptorTest.kt

@@ -0,0 +1,130 @@
+package com.fabledsword.minstrel.api
+
+import com.fabledsword.minstrel.auth.AuthStore
+import com.fabledsword.minstrel.cache.db.dao.AuthSessionDao
+import io.mockk.coEvery
+import io.mockk.every
+import io.mockk.mockk
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.flow.flowOf
+import kotlinx.coroutines.test.TestScope
+import kotlinx.coroutines.test.UnconfinedTestDispatcher
+import okhttp3.HttpUrl
+import okhttp3.OkHttpClient
+import okhttp3.Protocol
+import okhttp3.Request
+import okhttp3.Response
+import okhttp3.ResponseBody.Companion.toResponseBody
+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
+
+@OptIn(ExperimentalCoroutinesApi::class)
+class BaseUrlInterceptorTest {
+    private lateinit var server: MockWebServer
+    private lateinit var authStore: AuthStore
+
+    @BeforeEach
+    fun setup() {
+        server = MockWebServer().apply { start() }
+        val dao =
+            mockk<AuthSessionDao> {
+                every { observe() } returns flowOf(null)
+                coEvery { get() } returns null
+                coEvery { upsert(any()) } returns Unit
+                coEvery { setSessionCookie(any()) } returns Unit
+                coEvery { setBaseUrl(any()) } returns Unit
+            }
+        authStore = AuthStore(dao, TestScope(UnconfinedTestDispatcher()))
+    }
+
+    @AfterEach
+    fun teardown() {
+        server.shutdown()
+    }
+
+    @Test
+    fun `rewrites placeholder host to live baseUrl host port and scheme`() {
+        // baseUrl points at the mock server; outbound request targets the
+        // placeholder sentinel — interceptor should rewrite to the mock.
+        authStore.setBaseUrl(server.url("/").toString())
+        server.enqueue(MockResponse().setResponseCode(200))
+        val client = OkHttpClient.Builder().addInterceptor(BaseUrlInterceptor(authStore)).build()
+
+        client.newCall(
+            Request.Builder().url("http://placeholder.invalid/api/test").build(),
+        ).execute()
+
+        val recorded = server.takeRequest()
+        assertEquals("/api/test", recorded.path)
+        // MockWebServer's recorded Host header is the rewritten host:port
+        assertEquals("${server.hostName}:${server.port}", recorded.getHeader("Host"))
+    }
+
+    @Test
+    fun `passes external absolute URL through unchanged`() {
+        // Lidarr-surfaced artwork hosts (artwork.musicbrainz.org,
+        // coverartarchive.org) are absolute external URLs. The
+        // interceptor must NOT rewrite them to the Minstrel base —
+        // doing so produces 404s the user saw as missing Discover
+        // suggestion covers.
+        authStore.setBaseUrl(server.url("/").toString())
+        var seen: HttpUrl? = null
+        val client =
+            OkHttpClient.Builder()
+                .addInterceptor(BaseUrlInterceptor(authStore))
+                .addInterceptor { chain ->
+                    seen = chain.request().url
+                    Response.Builder()
+                        .request(chain.request())
+                        .protocol(Protocol.HTTP_1_1)
+                        .code(200)
+                        .message("OK")
+                        .body("".toResponseBody())
+                        .build()
+                }
+                .build()
+
+        client.newCall(
+            Request.Builder()
+                .url("https://artwork.musicbrainz.org/release-group/abc/front-250.jpg")
+                .build(),
+        ).execute()
+
+        assertEquals("artwork.musicbrainz.org", seen?.host)
+        assertEquals("https", seen?.scheme)
+        assertEquals("/release-group/abc/front-250.jpg", seen?.encodedPath)
+    }
+
+    @Test
+    fun `falls through when stored baseUrl is unparseable`() {
+        // Empty / malformed baseUrl can happen between cold start and
+        // first AuthStore load; the interceptor should leave the
+        // request alone rather than throw.
+        authStore.setBaseUrl("not a url")
+        var seen: HttpUrl? = null
+        val client =
+            OkHttpClient.Builder()
+                .addInterceptor(BaseUrlInterceptor(authStore))
+                .addInterceptor { chain ->
+                    seen = chain.request().url
+                    Response.Builder()
+                        .request(chain.request())
+                        .protocol(Protocol.HTTP_1_1)
+                        .code(200)
+                        .message("OK")
+                        .body("".toResponseBody())
+                        .build()
+                }
+                .build()
+
+        client.newCall(
+            Request.Builder().url("http://placeholder.invalid/api/test").build(),
+        ).execute()
+
+        assertEquals("placeholder.invalid", seen?.host)
+    }
+}

android/gradle/libs.versions.toml

@@ -51,6 +51,11 @@ compose-ui-graphics = { module = "androidx.compose.ui:ui-graphics" }
 compose-ui-tooling = { module = "androidx.compose.ui:ui-tooling" }
 compose-ui-tooling-preview = { module = "androidx.compose.ui:ui-tooling-preview" }
 compose-material3 = { module = "androidx.compose.material3:material3" }
+# material-icons-extended supplies the full Material Icons catalog
+# (Outlined / Filled / Rounded / Sharp / TwoTone). We use the Outlined
+# variant for LibraryMusic — its stroke style blends with Lucide.
+# Version pinned by compose-bom.
+compose-material-icons-extended = { module = "androidx.compose.material:material-icons-extended" }
 compose-ui-text-google-fonts = { module = "androidx.compose.ui:ui-text-google-fonts" }
 hilt-android = { module = "com.google.dagger:hilt-android", version.ref = "hilt" }
 hilt-compiler = { module = "com.google.dagger:hilt-compiler", version.ref = "hilt" }