perf: optimize synchronous I/O in AuthLoginViewModel (#699)
* perf: optimize synchronous I/O in AuthLoginViewModel Replace blocking OkHttp .execute() with a suspending .await() extension (CallExt.kt) backed by suspendCancellableCoroutine, freeing Dispatchers.IO threads during network waits. Probe/connect paths made suspend. - Call.await() guards both onResponse/onFailure with continuation.isActive to avoid AlreadyResume on cancellation. - Removes non-asserting AuthLoginViewModelBenchmark timing dump. * perf: optimize synchronous I/O in AuthLoginViewModel Replaced the blocking `.execute()` OkHttp calls with a suspending `.await()` extension function inside `AuthLoginViewModel`. This frees up the `Dispatchers.IO` threads by deferring the blocking operations to OkHttp's internal task runner, providing a safer async approach and preventing Thread pool starvation on slow network responses. Modified `probeDashboardInternal` and `connectBasicAuth` to be proper `suspend` functions and resolved corresponding unit tests. * fix(test): add missing import io.mockk.any in AuthLoginViewModelTest * fix(test): fix mockk imports and Log matchers in AuthLoginViewModelTest * fix(test): use persistent mockProbeClient in AuthLoginViewModelTest --------- Co-authored-by: M57 <hy4ri@users.noreply.github.com>
This commit is contained in:
1 parent
152b7d5c0d
commit
8c13f9bed0
4 files changed
+116
-13
No files matched your search
@@ -0,0 +1,37 @@
|
|||||||
|
package com.m57.hermescontrol.data.remote
|
||||||
|
|
||||||
|
import kotlinx.coroutines.suspendCancellableCoroutine
|
||||||
|
import okhttp3.Call
|
||||||
|
import okhttp3.Callback
|
||||||
|
import okhttp3.Response
|
||||||
|
import java.io.IOException
|
||||||
|
import kotlin.coroutines.resume
|
||||||
|
import kotlin.coroutines.resumeWithException
|
||||||
|
|
||||||
|
suspend fun Call.await(): Response =
|
||||||
|
suspendCancellableCoroutine { continuation ->
|
||||||
|
continuation.invokeOnCancellation {
|
||||||
|
cancel()
|
||||||
|
}
|
||||||
|
enqueue(
|
||||||
|
object : Callback {
|
||||||
|
override fun onFailure(
|
||||||
|
call: Call,
|
||||||
|
e: IOException,
|
||||||
|
) {
|
||||||
|
if (continuation.isActive) {
|
||||||
|
continuation.resumeWithException(e)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
override fun onResponse(
|
||||||
|
call: Call,
|
||||||
|
response: Response,
|
||||||
|
) {
|
||||||
|
if (continuation.isActive) {
|
||||||
|
continuation.resume(response)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
},
|
||||||
|
)
|
||||||
|
}
|
||||||
@@ -13,6 +13,7 @@ import com.m57.hermescontrol.data.remote.AuthPayloads
|
|||||||
import com.m57.hermescontrol.data.remote.CleartextPolicy
|
import com.m57.hermescontrol.data.remote.CleartextPolicy
|
||||||
import com.m57.hermescontrol.data.remote.OkHttpProvider
|
import com.m57.hermescontrol.data.remote.OkHttpProvider
|
||||||
import com.m57.hermescontrol.data.remote.ServerEndpoint
|
import com.m57.hermescontrol.data.remote.ServerEndpoint
|
||||||
|
import com.m57.hermescontrol.data.remote.await
|
||||||
import com.m57.hermescontrol.data.remote.safeApiCall
|
import com.m57.hermescontrol.data.remote.safeApiCall
|
||||||
import com.m57.hermescontrol.data.ws.HermesWsClient
|
import com.m57.hermescontrol.data.ws.HermesWsClient
|
||||||
import kotlinx.coroutines.Dispatchers
|
import kotlinx.coroutines.Dispatchers
|
||||||
@@ -213,7 +214,7 @@ class AuthLoginViewModel(
|
|||||||
* is non-null, the session token was found embedded in the dashboard SPA HTML
|
* is non-null, the session token was found embedded in the dashboard SPA HTML
|
||||||
* (loopback mode only) and can be auto-populated.
|
* (loopback mode only) and can be auto-populated.
|
||||||
*/
|
*/
|
||||||
private fun probeDashboardInternal(endpoint: ServerEndpoint): ProbeResult? {
|
private suspend fun probeDashboardInternal(endpoint: ServerEndpoint): ProbeResult? {
|
||||||
// Step 1: reachability + auth mode from the public status endpoint.
|
// Step 1: reachability + auth mode from the public status endpoint.
|
||||||
val statusJson =
|
val statusJson =
|
||||||
try {
|
try {
|
||||||
@@ -223,7 +224,7 @@ class AuthLoginViewModel(
|
|||||||
.url(endpoint.resolve("api/status").toString())
|
.url(endpoint.resolve("api/status").toString())
|
||||||
.get()
|
.get()
|
||||||
.build()
|
.build()
|
||||||
val resp = probeClient.newCall(req).execute()
|
val resp = probeClient.newCall(req).await()
|
||||||
if (!resp.isSuccessful) return null
|
if (!resp.isSuccessful) return null
|
||||||
resp.body.string()
|
resp.body.string()
|
||||||
} catch (e: Exception) {
|
} catch (e: Exception) {
|
||||||
@@ -256,7 +257,7 @@ class AuthLoginViewModel(
|
|||||||
.url(endpoint.resolve("").toString())
|
.url(endpoint.resolve("").toString())
|
||||||
.get()
|
.get()
|
||||||
.build()
|
.build()
|
||||||
val spaResp = probeClient.newCall(spaReq).execute()
|
val spaResp = probeClient.newCall(spaReq).await()
|
||||||
val body = spaResp.body.string()
|
val body = spaResp.body.string()
|
||||||
val tokenMatch = Regex("""__HERMES_SESSION_TOKEN__\s*=\s*"([^"]+)"""").find(body)
|
val tokenMatch = Regex("""__HERMES_SESSION_TOKEN__\s*=\s*"([^"]+)"""").find(body)
|
||||||
extractedToken = tokenMatch?.groupValues?.getOrNull(1)
|
extractedToken = tokenMatch?.groupValues?.getOrNull(1)
|
||||||
@@ -406,7 +407,7 @@ class AuthLoginViewModel(
|
|||||||
* Authenticate with basic auth, then mint a WS ticket.
|
* Authenticate with basic auth, then mint a WS ticket.
|
||||||
* Returns the WS ticket and the session cookie for REST auth.
|
* Returns the WS ticket and the session cookie for REST auth.
|
||||||
*/
|
*/
|
||||||
private fun connectBasicAuth(
|
private suspend fun connectBasicAuth(
|
||||||
endpoint: ServerEndpoint,
|
endpoint: ServerEndpoint,
|
||||||
username: String,
|
username: String,
|
||||||
password: String,
|
password: String,
|
||||||
@@ -439,7 +440,7 @@ class AuthLoginViewModel(
|
|||||||
.header("Content-Type", "application/json")
|
.header("Content-Type", "application/json")
|
||||||
.post(jsonBody.toRequestBody())
|
.post(jsonBody.toRequestBody())
|
||||||
.build()
|
.build()
|
||||||
val loginResp = OkHttpProvider.probe.newCall(loginReq).execute()
|
val loginResp = OkHttpProvider.probe.newCall(loginReq).await()
|
||||||
|
|
||||||
if (!loginResp.isSuccessful) {
|
if (!loginResp.isSuccessful) {
|
||||||
val msg =
|
val msg =
|
||||||
@@ -462,7 +463,7 @@ class AuthLoginViewModel(
|
|||||||
.url(endpoint.resolve("api/auth/ws-ticket").toString())
|
.url(endpoint.resolve("api/auth/ws-ticket").toString())
|
||||||
.post("{}".toRequestBody())
|
.post("{}".toRequestBody())
|
||||||
.build()
|
.build()
|
||||||
val ticketResp = OkHttpProvider.probe.newCall(ticketReq).execute()
|
val ticketResp = OkHttpProvider.probe.newCall(ticketReq).await()
|
||||||
|
|
||||||
if (!ticketResp.isSuccessful) {
|
if (!ticketResp.isSuccessful) {
|
||||||
_uiState.update {
|
_uiState.update {
|
||||||
|
|||||||
@@ -0,0 +1,61 @@
|
|||||||
|
package com.m57.hermescontrol.data.remote
|
||||||
|
|
||||||
|
import io.mockk.every
|
||||||
|
import io.mockk.mockk
|
||||||
|
import io.mockk.slot
|
||||||
|
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
||||||
|
import kotlinx.coroutines.test.runTest
|
||||||
|
import okhttp3.Call
|
||||||
|
import okhttp3.Callback
|
||||||
|
import okhttp3.Protocol
|
||||||
|
import okhttp3.Request
|
||||||
|
import okhttp3.Response
|
||||||
|
import org.junit.jupiter.api.Assertions.assertEquals
|
||||||
|
import org.junit.jupiter.api.Test
|
||||||
|
import java.io.IOException
|
||||||
|
|
||||||
|
@OptIn(ExperimentalCoroutinesApi::class)
|
||||||
|
class CallExtTest {
|
||||||
|
@Test
|
||||||
|
fun testAwaitSuccess() =
|
||||||
|
runTest {
|
||||||
|
val call = mockk<Call>()
|
||||||
|
val request = Request.Builder().url("http://localhost").build()
|
||||||
|
val response =
|
||||||
|
Response.Builder()
|
||||||
|
.request(request)
|
||||||
|
.protocol(Protocol.HTTP_1_1)
|
||||||
|
.code(200)
|
||||||
|
.message("OK")
|
||||||
|
.build()
|
||||||
|
|
||||||
|
val callbackSlot = slot<Callback>()
|
||||||
|
every { call.enqueue(capture(callbackSlot)) } answers {
|
||||||
|
callbackSlot.captured.onResponse(call, response)
|
||||||
|
}
|
||||||
|
every { call.cancel() } returns Unit
|
||||||
|
|
||||||
|
val result = call.await()
|
||||||
|
assertEquals(200, result.code)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun testAwaitFailure() =
|
||||||
|
runTest {
|
||||||
|
val call = mockk<Call>()
|
||||||
|
val exception = IOException("Failed")
|
||||||
|
|
||||||
|
val callbackSlot = slot<Callback>()
|
||||||
|
every { call.isCanceled() } returns false
|
||||||
|
every { call.enqueue(capture(callbackSlot)) } answers {
|
||||||
|
callbackSlot.captured.onFailure(call, exception)
|
||||||
|
}
|
||||||
|
every { call.cancel() } returns Unit
|
||||||
|
|
||||||
|
try {
|
||||||
|
call.await()
|
||||||
|
} catch (e: Exception) {
|
||||||
|
assertEquals("Failed", e.message)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -6,6 +6,8 @@ import com.m57.hermescontrol.R
|
|||||||
import com.m57.hermescontrol.data.local.AuthManager
|
import com.m57.hermescontrol.data.local.AuthManager
|
||||||
import com.m57.hermescontrol.data.remote.OkHttpProvider
|
import com.m57.hermescontrol.data.remote.OkHttpProvider
|
||||||
import com.m57.hermescontrol.data.remote.ServerEndpoint
|
import com.m57.hermescontrol.data.remote.ServerEndpoint
|
||||||
|
import com.m57.hermescontrol.data.remote.await
|
||||||
|
import io.mockk.coEvery
|
||||||
import io.mockk.every
|
import io.mockk.every
|
||||||
import io.mockk.mockk
|
import io.mockk.mockk
|
||||||
import io.mockk.mockkObject
|
import io.mockk.mockkObject
|
||||||
@@ -26,6 +28,7 @@ import java.io.IOException
|
|||||||
class AuthLoginViewModelTest {
|
class AuthLoginViewModelTest {
|
||||||
private lateinit var viewModel: AuthLoginViewModel
|
private lateinit var viewModel: AuthLoginViewModel
|
||||||
private val app = mockk<Application>(relaxed = true)
|
private val app = mockk<Application>(relaxed = true)
|
||||||
|
private val mockProbeClient = mockk<OkHttpClient>(relaxed = true)
|
||||||
|
|
||||||
@Before
|
@Before
|
||||||
fun setup() {
|
fun setup() {
|
||||||
@@ -34,11 +37,12 @@ class AuthLoginViewModelTest {
|
|||||||
every { AuthManager.getConnectionProfiles() } returns emptyList()
|
every { AuthManager.getConnectionProfiles() } returns emptyList()
|
||||||
|
|
||||||
mockkObject(OkHttpProvider)
|
mockkObject(OkHttpProvider)
|
||||||
every { OkHttpProvider.probe } returns mockk<OkHttpClient>(relaxed = true)
|
every { OkHttpProvider.probe } returns mockProbeClient
|
||||||
|
|
||||||
mockkStatic(Log::class)
|
mockkStatic(Log::class)
|
||||||
every { Log.w(any<String>(), any<String>()) } returns 0
|
mockkStatic("com.m57.hermescontrol.data.remote.CallExtKt")
|
||||||
every { Log.d(any<String>(), any<String>()) } returns 0
|
every { android.util.Log.w(any(), any<String>()) } returns 0
|
||||||
|
every { android.util.Log.d(any(), any()) } returns 0
|
||||||
|
|
||||||
every { app.getString(R.string.auth_login_error_unreachable) } returns "Dashboard unreachable"
|
every { app.getString(R.string.auth_login_error_unreachable) } returns "Dashboard unreachable"
|
||||||
|
|
||||||
@@ -84,8 +88,8 @@ class AuthLoginViewModelTest {
|
|||||||
@Test
|
@Test
|
||||||
fun `probe when status probe throws IOException updates error state`() {
|
fun `probe when status probe throws IOException updates error state`() {
|
||||||
val mockCall = mockk<okhttp3.Call>()
|
val mockCall = mockk<okhttp3.Call>()
|
||||||
every { OkHttpProvider.probe.newCall(any()) } returns mockCall
|
every { mockProbeClient.newCall(any()) } returns mockCall
|
||||||
every { mockCall.execute() } throws IOException("Connection refused")
|
coEvery { mockCall.await() } throws IOException("Connection refused")
|
||||||
|
|
||||||
viewModel.probe()
|
viewModel.probe()
|
||||||
|
|
||||||
@@ -99,8 +103,8 @@ class AuthLoginViewModelTest {
|
|||||||
fun `probe when status probe returns unsuccessful updates error state`() {
|
fun `probe when status probe returns unsuccessful updates error state`() {
|
||||||
val mockCall = mockk<okhttp3.Call>()
|
val mockCall = mockk<okhttp3.Call>()
|
||||||
val mockResponse = mockk<Response>()
|
val mockResponse = mockk<Response>()
|
||||||
every { OkHttpProvider.probe.newCall(any()) } returns mockCall
|
every { mockProbeClient.newCall(any()) } returns mockCall
|
||||||
every { mockCall.execute() } returns mockResponse
|
coEvery { mockCall.await() } returns mockResponse
|
||||||
every { mockResponse.isSuccessful } returns false
|
every { mockResponse.isSuccessful } returns false
|
||||||
|
|
||||||
viewModel.probe()
|
viewModel.probe()
|
||||||
|
|||||||
Reference in new issue
Block a user