diff --git a/android/app/src/main/java/com/fabledsword/minstrel/api/BaseUrlInterceptor.kt b/android/app/src/main/java/com/fabledsword/minstrel/api/BaseUrlInterceptor.kt index 79f838d4..06e59bc2 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/api/BaseUrlInterceptor.kt +++ b/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 - * the live AuthStore value. Lets the user change server URL in - * Settings without an app relaunch (Phase 11 wiring). + * placeholder forever, and every actual Minstrel-bound request gets + * retargeted at the live AuthStore value. Lets the user change + * 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,6 +38,7 @@ class BaseUrlInterceptor @Inject constructor( override fun intercept(chain: Interceptor.Chain): Response { val original = chain.request() + if (original.url.host != PLACEHOLDER_HOST) return chain.proceed(original) val baseUrl = authStore.baseUrl.value.toHttpUrlOrNull() ?: return chain.proceed(original) val rewritten: HttpUrl = original.url.newBuilder() @@ -40,4 +48,12 @@ class BaseUrlInterceptor @Inject constructor( .build() 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" + } } diff --git a/android/app/src/test/java/com/fabledsword/minstrel/api/BaseUrlInterceptorTest.kt b/android/app/src/test/java/com/fabledsword/minstrel/api/BaseUrlInterceptorTest.kt new file mode 100644 index 00000000..a5771f9a --- /dev/null +++ b/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 { + 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) + } +}