diff --git a/composeApp/src/commonMain/kotlin/com/nuvio/app/features/simkl/SimklApiClient.kt b/composeApp/src/commonMain/kotlin/com/nuvio/app/features/simkl/SimklApiClient.kt new file mode 100644 index 00000000..47ed55aa --- /dev/null +++ b/composeApp/src/commonMain/kotlin/com/nuvio/app/features/simkl/SimklApiClient.kt @@ -0,0 +1,245 @@ +package com.nuvio.app.features.simkl + +import com.nuvio.app.features.addons.RawHttpResponse +import com.nuvio.app.features.addons.httpRequestRaw +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.delay +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock +import kotlinx.serialization.Serializable +import kotlinx.serialization.decodeFromString +import kotlinx.serialization.json.Json +import kotlin.math.max +import kotlin.random.Random + +private const val SIMKL_MAX_RESPONSE_BODY_BYTES = 8 * 1024 * 1024 + +internal enum class SimklHttpMethod { + GET, + POST, + DELETE, +} + +internal data class SimklApiRequest( + val method: SimklHttpMethod, + val path: String, + val query: Map = emptyMap(), + val body: String = "", + val requiresAuthentication: Boolean = true, + val scrobbleStopConflictIsSuccess: Boolean = false, +) + +internal data class SimklApiResponse( + val status: Int, + val body: String, + val headers: Map, + val isSoftSuccess: Boolean = false, +) + +internal class SimklApiException( + val status: Int?, + val errorCode: String?, + override val message: String, + cause: Throwable? = null, +) : Exception(message, cause) + +internal fun interface SimklHttpEngine { + suspend fun execute( + method: String, + url: String, + headers: Map, + body: String, + ): RawHttpResponse +} + +internal class SimklApiClient( + private val engine: SimklHttpEngine, + private val accessToken: () -> String?, + private val onUnauthorized: () -> Unit, + private val nowEpochMs: () -> Long = SimklPlatformClock::nowEpochMs, + private val sleep: suspend (Long) -> Unit = { delayMs -> delay(delayMs) }, + private val retryJitterMs: () -> Long = { Random.nextLong(RETRY_JITTER_BOUND_MS + 1L) }, +) { + private val requestMutex = Mutex() + private val json = Json { ignoreUnknownKeys = true } + private var nextGetAtEpochMs = 0L + private var nextPostAtEpochMs = 0L + + suspend fun execute(request: SimklApiRequest): SimklApiResponse = requestMutex.withLock { + val token = if (request.requiresAuthentication) { + accessToken()?.takeIf(String::isNotBlank) + ?: throw SimklApiException( + status = 401, + errorCode = "authentication_required", + message = "Simkl authentication is required", + ) + } else { + null + } + + var lastTransportFailure: Throwable? = null + for (attempt in 0..MAX_RETRIES) { + awaitRateLimit(request.method) + val response = try { + engine.execute( + method = request.method.name, + url = buildSimklApiUrl(request.path, request.query), + headers = simklRequestHeaders( + accessToken = token, + contentTypeJson = request.body.isNotEmpty(), + ), + body = request.body, + ) + } catch (error: CancellationException) { + throw error + } catch (error: Throwable) { + lastTransportFailure = error + if (attempt == MAX_RETRIES) { + throw SimklApiException( + status = null, + errorCode = "transport_failure", + message = "Simkl request failed", + cause = error, + ) + } + sleep(retryDelayMs(attempt, retryAfterHeader = null, retryJitterMs())) + continue + } + + when (classifySimklResponse(response.status, request.scrobbleStopConflictIsSuccess)) { + SimklResponseAction.SUCCESS -> return@withLock response.toApiResponse() + SimklResponseAction.SOFT_SUCCESS -> { + return@withLock response.toApiResponse(isSoftSuccess = true) + } + SimklResponseAction.REAUTHENTICATE -> { + onUnauthorized() + throw response.toApiException(json) + } + SimklResponseAction.FAIL -> throw response.toApiException(json) + SimklResponseAction.RETRY -> { + if (attempt == MAX_RETRIES) throw response.toApiException(json) + sleep( + retryDelayMs( + attempt = attempt, + retryAfterHeader = response.headers.headerValue("retry-after"), + jitterMs = retryJitterMs(), + ), + ) + } + } + } + + throw SimklApiException( + status = null, + errorCode = "transport_failure", + message = "Simkl request failed", + cause = lastTransportFailure, + ) + } + + private suspend fun awaitRateLimit(method: SimklHttpMethod) { + val now = nowEpochMs() + val scheduledAt = when (method) { + SimklHttpMethod.GET -> max(now, nextGetAtEpochMs) + SimklHttpMethod.POST, SimklHttpMethod.DELETE -> max(now, nextPostAtEpochMs) + } + if (scheduledAt > now) sleep(scheduledAt - now) + val requestAt = max(scheduledAt, nowEpochMs()) + when (method) { + SimklHttpMethod.GET -> nextGetAtEpochMs = requestAt + GET_INTERVAL_MS + SimklHttpMethod.POST, SimklHttpMethod.DELETE -> nextPostAtEpochMs = requestAt + POST_INTERVAL_MS + } + } + + private companion object { + const val GET_INTERVAL_MS = 100L + const val POST_INTERVAL_MS = 1_000L + const val MAX_RETRIES = 5 + const val RETRY_JITTER_BOUND_MS = 1_000L + } +} + +internal object SimklApi { + val client: SimklApiClient by lazy { + SimklApiClient( + engine = SimklHttpEngine { method, url, headers, body -> + httpRequestRaw( + method = method, + url = url, + headers = headers, + body = body, + maxResponseBodyBytes = SIMKL_MAX_RESPONSE_BODY_BYTES, + ) + }, + accessToken = SimklAuthRepository::authorizedAccessToken, + onUnauthorized = SimklAuthRepository::onUnauthorizedResponse, + ) + } +} + +internal enum class SimklResponseAction { + SUCCESS, + SOFT_SUCCESS, + REAUTHENTICATE, + RETRY, + FAIL, +} + +internal fun classifySimklResponse( + status: Int, + scrobbleStopConflictIsSuccess: Boolean = false, +): SimklResponseAction = when { + status in 200..299 -> SimklResponseAction.SUCCESS + status == 409 && scrobbleStopConflictIsSuccess -> SimklResponseAction.SOFT_SUCCESS + status == 401 -> SimklResponseAction.REAUTHENTICATE + status == 429 || status == 500 || status == 502 || status == 503 -> SimklResponseAction.RETRY + else -> SimklResponseAction.FAIL +} + +internal fun retryDelayMs( + attempt: Int, + retryAfterHeader: String?, + jitterMs: Long, +): Long { + require(attempt in 0..4) { "Retry attempt must be between 0 and 4" } + val exponentialDelayMs = 1_000L shl attempt + val retryAfterMs = retryAfterHeader + ?.substringBefore(',') + ?.trim() + ?.toLongOrNull() + ?.coerceAtLeast(0L) + ?.times(1_000L) + ?: 0L + return max(exponentialDelayMs, retryAfterMs) + jitterMs.coerceIn(0L, 1_000L) +} + +private fun RawHttpResponse.toApiResponse(isSoftSuccess: Boolean = false): SimklApiResponse = + SimklApiResponse( + status = status, + body = body, + headers = headers, + isSoftSuccess = isSoftSuccess, + ) + +private fun RawHttpResponse.toApiException(json: Json): SimklApiException { + val envelope = body.takeIf(String::isNotBlank)?.let { payload -> + runCatching { json.decodeFromString(payload) }.getOrNull() + } + return SimklApiException( + status = status, + errorCode = envelope?.error, + message = envelope?.message?.takeIf(String::isNotBlank) + ?: envelope?.error?.takeIf(String::isNotBlank) + ?: "Simkl request failed with HTTP $status", + ) +} + +private fun Map.headerValue(name: String): String? = + entries.firstOrNull { (key, _) -> key.equals(name, ignoreCase = true) }?.value + +@Serializable +private data class SimklErrorEnvelope( + val error: String? = null, + val code: Int? = null, + val message: String? = null, +) diff --git a/composeApp/src/commonMain/kotlin/com/nuvio/app/features/simkl/SimklApiMetadata.kt b/composeApp/src/commonMain/kotlin/com/nuvio/app/features/simkl/SimklApiMetadata.kt index a76bd9f7..99d1be26 100644 --- a/composeApp/src/commonMain/kotlin/com/nuvio/app/features/simkl/SimklApiMetadata.kt +++ b/composeApp/src/commonMain/kotlin/com/nuvio/app/features/simkl/SimklApiMetadata.kt @@ -16,11 +16,12 @@ internal fun buildSimklApiUrl( val normalizedPath = path.trim().let { value -> if (value.startsWith('/')) value else "/$value" } - val parameters = linkedMapOf( - "client_id" to SimklConfig.CLIENT_ID, - "app-name" to SimklConfig.APP_NAME, - "app-version" to simklAppVersion, - ).apply { putAll(query) } + val parameters = linkedMapOf().apply { + putAll(query) + put("client_id", SimklConfig.CLIENT_ID) + put("app-name", SimklConfig.APP_NAME) + put("app-version", simklAppVersion) + } return buildString { append(SIMKL_API_BASE_URL) append(normalizedPath) @@ -38,6 +39,7 @@ internal fun simklRequestHeaders( contentTypeJson: Boolean = false, ): Map = buildMap { put("User-Agent", "${SimklConfig.APP_NAME}/$simklAppVersion") + put("Accept", "application/json") accessToken?.trim()?.takeIf(String::isNotBlank)?.let { token -> put("Authorization", "Bearer $token") } diff --git a/composeApp/src/commonMain/kotlin/com/nuvio/app/features/simkl/SimklAuthRepository.kt b/composeApp/src/commonMain/kotlin/com/nuvio/app/features/simkl/SimklAuthRepository.kt index ada9d59f..337d6968 100644 --- a/composeApp/src/commonMain/kotlin/com/nuvio/app/features/simkl/SimklAuthRepository.kt +++ b/composeApp/src/commonMain/kotlin/com/nuvio/app/features/simkl/SimklAuthRepository.kt @@ -185,8 +185,8 @@ object SimklAuthRepository : TrackingAuthProvider { } suspend fun refreshUserSettings(): String? { - val token = authorizedAccessToken() ?: return null - return fetchAndStoreUserSettings(token) + authorizedAccessToken() ?: return null + return fetchAndStoreUserSettings() } private suspend fun completeAuthorization(callback: SimklAuthCallback.AuthorizationCode) = @@ -260,16 +260,17 @@ object SimklAuthRepository : TrackingAuthProvider { ) persistMetadata() publish(isLoading = false, error = null) - fetchAndStoreUserSettings(token.accessToken) + fetchAndStoreUserSettings() } - private suspend fun fetchAndStoreUserSettings(token: String): String? { + private suspend fun fetchAndStoreUserSettings(): String? { val response = try { - httpRequestRaw( - method = "POST", - url = buildSimklApiUrl("/users/settings"), - headers = simklRequestHeaders(accessToken = token, contentTypeJson = true), - body = "{}", + SimklApi.client.execute( + SimklApiRequest( + method = SimklHttpMethod.POST, + path = "/users/settings", + body = "{}", + ), ) } catch (error: CancellationException) { throw error @@ -277,11 +278,6 @@ object SimklAuthRepository : TrackingAuthProvider { log.w { "Failed to fetch Simkl user settings: ${error.message}" } return null } - if (response.status == 401) { - onUnauthorizedResponse() - return null - } - if (response.status !in 200..299) return null val settings = runCatching { json.decodeFromString(response.body) } .getOrNull() ?: return null storedState = storedState.copy( diff --git a/composeApp/src/commonTest/kotlin/com/nuvio/app/features/simkl/SimklApiClientTest.kt b/composeApp/src/commonTest/kotlin/com/nuvio/app/features/simkl/SimklApiClientTest.kt new file mode 100644 index 00000000..bf5edc94 --- /dev/null +++ b/composeApp/src/commonTest/kotlin/com/nuvio/app/features/simkl/SimklApiClientTest.kt @@ -0,0 +1,196 @@ +package com.nuvio.app.features.simkl + +import com.nuvio.app.features.addons.RawHttpResponse +import kotlinx.coroutines.runBlocking +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class SimklApiClientTest { + @Test + fun `response classification retries only documented transient statuses`() { + assertEquals(SimklResponseAction.SUCCESS, classifySimklResponse(200)) + assertEquals(SimklResponseAction.REAUTHENTICATE, classifySimklResponse(401)) + assertEquals(SimklResponseAction.RETRY, classifySimklResponse(429)) + assertEquals(SimklResponseAction.RETRY, classifySimklResponse(500)) + assertEquals(SimklResponseAction.RETRY, classifySimklResponse(502)) + assertEquals(SimklResponseAction.RETRY, classifySimklResponse(503)) + assertEquals(SimklResponseAction.FAIL, classifySimklResponse(400)) + assertEquals(SimklResponseAction.FAIL, classifySimklResponse(409)) + assertEquals(SimklResponseAction.SOFT_SUCCESS, classifySimklResponse(409, true)) + assertEquals(SimklResponseAction.FAIL, classifySimklResponse(412)) + } + + @Test + fun `retry schedule uses exponential backoff and respects retry after`() { + assertEquals(1_000L, retryDelayMs(0, null, 0L)) + assertEquals(2_000L, retryDelayMs(1, null, 0L)) + assertEquals(4_000L, retryDelayMs(2, null, 0L)) + assertEquals(8_000L, retryDelayMs(3, null, 0L)) + assertEquals(16_000L, retryDelayMs(4, null, 0L)) + assertEquals(30_250L, retryDelayMs(0, "30", 250L)) + } + + @Test + fun `callers cannot override mandatory application metadata`() { + val url = buildSimklApiUrl( + path = "/sync/activities", + query = mapOf( + "client_id" to "spoofed", + "app-name" to "spoofed", + "app-version" to "spoofed", + ), + ) + + assertFalse("spoofed" in url) + } + + @Test + fun `every request carries metadata query and required headers`() = runBlocking { + val engine = RecordingEngine(response(200)) + val harness = TestHarness(engine) + + harness.client.execute( + SimklApiRequest( + method = SimklHttpMethod.GET, + path = "/sync/activities", + ), + ) + + val request = engine.requests.single() + assertTrue("client_id=" in request.url) + assertTrue("app-name=" in request.url) + assertTrue("app-version=" in request.url) + assertEquals("Bearer token", request.headers["Authorization"]) + assertEquals("application/json", request.headers["Accept"]) + assertTrue(request.headers.getValue("User-Agent").contains('/')) + } + + @Test + fun `authenticated requests are serialized at documented method rates`() = runBlocking { + val engine = RecordingEngine(response(200), response(200), response(200), response(200)) + val harness = TestHarness(engine) + + harness.client.execute(SimklApiRequest(SimklHttpMethod.GET, "/one")) + harness.client.execute(SimklApiRequest(SimklHttpMethod.GET, "/two")) + harness.client.execute(SimklApiRequest(SimklHttpMethod.POST, "/three", body = "{}")) + harness.client.execute(SimklApiRequest(SimklHttpMethod.POST, "/four", body = "{}")) + + assertEquals(listOf(100L, 1_000L), harness.sleeps) + assertEquals(listOf(0L, 100L, 100L, 1_100L), engine.requests.map { it.atEpochMs }) + } + + @Test + fun `transient responses retry sequentially and deterministic errors do not`() = runBlocking { + val transientEngine = RecordingEngine(response(503), response(502), response(200)) + val transientHarness = TestHarness(transientEngine) + + transientHarness.client.execute(SimklApiRequest(SimklHttpMethod.GET, "/retry")) + + assertEquals(listOf(1_000L, 2_000L), transientHarness.sleeps) + assertEquals(3, transientEngine.requests.size) + + val deterministicEngine = RecordingEngine(response(400, """{"error":"wrong_parameter","code":400}""")) + val deterministicHarness = TestHarness(deterministicEngine) + val error = assertFailsWith { + deterministicHarness.client.execute(SimklApiRequest(SimklHttpMethod.POST, "/bad", body = "{}")) + } + assertEquals("wrong_parameter", error.errorCode) + assertEquals(1, deterministicEngine.requests.size) + } + + @Test + fun `retry after and unauthorized handling are applied once`() = runBlocking { + val retryEngine = RecordingEngine( + response(429, headers = mapOf("Retry-After" to "3")), + response(200), + ) + val retryHarness = TestHarness(retryEngine) + retryHarness.client.execute(SimklApiRequest(SimklHttpMethod.GET, "/limited")) + assertEquals(listOf(3_000L), retryHarness.sleeps) + + val unauthorizedEngine = RecordingEngine(response(401)) + val unauthorizedHarness = TestHarness(unauthorizedEngine) + assertFailsWith { + unauthorizedHarness.client.execute(SimklApiRequest(SimklHttpMethod.GET, "/private")) + } + assertTrue(unauthorizedHarness.wasUnauthorized) + assertEquals(1, unauthorizedEngine.requests.size) + } + + @Test + fun `duplicate scrobble stop is a soft success`() = runBlocking { + val engine = RecordingEngine(response(409)) + val harness = TestHarness(engine) + + val result = harness.client.execute( + SimklApiRequest( + method = SimklHttpMethod.POST, + path = "/scrobble/stop", + body = "{}", + scrobbleStopConflictIsSuccess = true, + ), + ) + + assertEquals(409, result.status) + assertTrue(result.isSoftSuccess) + assertFalse(harness.wasUnauthorized) + } + + private class TestHarness(engine: RecordingEngine) { + var now = 0L + val sleeps = mutableListOf() + var wasUnauthorized = false + val client = SimklApiClient( + engine = engine.also { recording -> recording.now = { now } }, + accessToken = { "token" }, + onUnauthorized = { wasUnauthorized = true }, + nowEpochMs = { now }, + sleep = { delayMs -> + sleeps += delayMs + now += delayMs + }, + retryJitterMs = { 0L }, + ) + } + + private class RecordingEngine(vararg responses: RawHttpResponse) : SimklHttpEngine { + private val queuedResponses = responses.toMutableList() + val requests = mutableListOf() + var now: () -> Long = { 0L } + + override suspend fun execute( + method: String, + url: String, + headers: Map, + body: String, + ): RawHttpResponse { + requests += RecordedRequest(method, url, headers, body, now()) + return queuedResponses.removeAt(0) + } + } + + private data class RecordedRequest( + val method: String, + val url: String, + val headers: Map, + val body: String, + val atEpochMs: Long, + ) + + private companion object { + fun response( + status: Int, + body: String = "{}", + headers: Map = emptyMap(), + ) = RawHttpResponse( + status = status, + statusText = "", + url = "https://api.simkl.com/test", + body = body, + headers = headers, + ) + } +}