Merge pull request #1168 from Hy4ri/fix/1164-screen-load-rpc
refactor(#1164): use WS for compatible model inventory reads
This commit is contained in:
5 files changed
+158
-5
No files matched your search
@@ -0,0 +1,50 @@
|
|||||||
|
package com.m57.hermescontrol.data.ws
|
||||||
|
|
||||||
|
import com.m57.hermescontrol.data.model.ModelOptionsResponse
|
||||||
|
import com.m57.hermescontrol.data.remote.ApiClient
|
||||||
|
import com.m57.hermescontrol.data.remote.NetworkResult
|
||||||
|
import com.m57.hermescontrol.data.remote.OkHttpProvider
|
||||||
|
import com.m57.hermescontrol.data.remote.safeApiCall
|
||||||
|
import kotlinx.coroutines.CancellationException
|
||||||
|
import kotlinx.coroutines.currentCoroutineContext
|
||||||
|
import kotlinx.coroutines.ensureActive
|
||||||
|
import kotlinx.serialization.json.JsonElement
|
||||||
|
import kotlinx.serialization.json.decodeFromJsonElement
|
||||||
|
|
||||||
|
/** Issue #1164: profile-configured inventory; never pass a live chat session override. */
|
||||||
|
class ModelOptionsRepository(
|
||||||
|
private val connected: () -> Boolean = {
|
||||||
|
HermesWsClient.connectionStatus.value == ConnectionStatus.CONNECTED
|
||||||
|
},
|
||||||
|
private val request: suspend (Map<String, Any>) -> Any? = { params ->
|
||||||
|
val deferred = HermesWsClient.request(WsMethods.MODEL_OPTIONS, params)
|
||||||
|
try {
|
||||||
|
deferred.await()
|
||||||
|
} finally {
|
||||||
|
if (!deferred.isCompleted) deferred.cancel()
|
||||||
|
}
|
||||||
|
},
|
||||||
|
private val rest: suspend (Boolean) -> NetworkResult<ModelOptionsResponse> = { refresh ->
|
||||||
|
safeApiCall { ApiClient.hermesApi.getModelOptions(refresh = refresh, includeUnconfigured = false) }
|
||||||
|
},
|
||||||
|
) {
|
||||||
|
suspend fun load(refresh: Boolean = false): NetworkResult<ModelOptionsResponse> {
|
||||||
|
currentCoroutineContext().ensureActive()
|
||||||
|
if (connected()) {
|
||||||
|
try {
|
||||||
|
val result = request(mapOf("refresh" to refresh, "include_unconfigured" to false))
|
||||||
|
// request() returns raw JsonElement, unlike the pushed-event Map representation.
|
||||||
|
val json = result as? JsonElement ?: error("Invalid model.options result")
|
||||||
|
return NetworkResult.Success(OkHttpProvider.json.decodeFromJsonElement<ModelOptionsResponse>(json))
|
||||||
|
} catch (e: CancellationException) {
|
||||||
|
// A disconnected socket may cancel the independent RPC deferred. Only fall back
|
||||||
|
// if the screen's own coroutine is still active; never swallow caller cancellation.
|
||||||
|
currentCoroutineContext().ensureActive()
|
||||||
|
} catch (_: Exception) {
|
||||||
|
// Older gateways and malformed payloads keep the existing HTTP behavior.
|
||||||
|
}
|
||||||
|
}
|
||||||
|
currentCoroutineContext().ensureActive()
|
||||||
|
return rest(refresh)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -28,6 +28,7 @@ object WsMethods {
|
|||||||
const val SESSION_USAGE = "session.usage"
|
const val SESSION_USAGE = "session.usage"
|
||||||
const val PROFILES_CONFIGURE = "profiles.configure"
|
const val PROFILES_CONFIGURE = "profiles.configure"
|
||||||
const val PROFILES_LIST = "profiles.list"
|
const val PROFILES_LIST = "profiles.list"
|
||||||
|
const val MODEL_OPTIONS = "model.options"
|
||||||
|
|
||||||
// ── Projects ──────────────────────────────────────────────────────────
|
// ── Projects ──────────────────────────────────────────────────────────
|
||||||
|
|
||||||
@@ -154,6 +155,7 @@ object WsMethods {
|
|||||||
// Evidence: methods_session.py lines 42, 164, 232, 317, 845
|
// Evidence: methods_session.py lines 42, 164, 232, 317, 845
|
||||||
SESSION_CREATE, // session.create
|
SESSION_CREATE, // session.create
|
||||||
SESSION_LIST, // session.list
|
SESSION_LIST, // session.list
|
||||||
|
MODEL_OPTIONS, // model.options: @_profile_scoped
|
||||||
SESSION_RESUME, // session.resume
|
SESSION_RESUME, // session.resume
|
||||||
SESSION_DELETE, // session.delete
|
SESSION_DELETE, // session.delete
|
||||||
SESSION_STATUS, // session.status
|
SESSION_STATUS, // session.status
|
||||||
|
|||||||
@@ -13,6 +13,7 @@ import com.m57.hermescontrol.data.model.UpdateProfileModelRequest
|
|||||||
import com.m57.hermescontrol.data.remote.ApiClient
|
import com.m57.hermescontrol.data.remote.ApiClient
|
||||||
import com.m57.hermescontrol.data.remote.NetworkResult
|
import com.m57.hermescontrol.data.remote.NetworkResult
|
||||||
import com.m57.hermescontrol.data.remote.safeApiCall
|
import com.m57.hermescontrol.data.remote.safeApiCall
|
||||||
|
import com.m57.hermescontrol.data.ws.ModelOptionsRepository
|
||||||
import com.m57.hermescontrol.ui.common.ToastHost
|
import com.m57.hermescontrol.ui.common.ToastHost
|
||||||
import kotlinx.coroutines.Dispatchers
|
import kotlinx.coroutines.Dispatchers
|
||||||
import kotlinx.coroutines.async
|
import kotlinx.coroutines.async
|
||||||
@@ -66,8 +67,9 @@ data class ModelUiState(
|
|||||||
val pendingModelPickerResolve: ((Boolean) -> Unit)? = null,
|
val pendingModelPickerResolve: ((Boolean) -> Unit)? = null,
|
||||||
)
|
)
|
||||||
|
|
||||||
class ModelViewModel :
|
class ModelViewModel(
|
||||||
ViewModel(),
|
private val modelOptionsRepository: ModelOptionsRepository = ModelOptionsRepository(),
|
||||||
|
) : ViewModel(),
|
||||||
ToastHost {
|
ToastHost {
|
||||||
private val _uiState = MutableStateFlow(ModelUiState())
|
private val _uiState = MutableStateFlow(ModelUiState())
|
||||||
val uiState: StateFlow<ModelUiState> = _uiState.asStateFlow()
|
val uiState: StateFlow<ModelUiState> = _uiState.asStateFlow()
|
||||||
@@ -100,7 +102,7 @@ class ModelViewModel :
|
|||||||
// Phase 2: Launch model options concurrently
|
// Phase 2: Launch model options concurrently
|
||||||
val optionsDeferred =
|
val optionsDeferred =
|
||||||
async(Dispatchers.IO) {
|
async(Dispatchers.IO) {
|
||||||
safeApiCall { ApiClient.hermesApi.getModelOptions(refresh = refresh, includeUnconfigured = false) }
|
modelOptionsRepository.load(refresh)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Await Phase 1 calls (~40-190ms total) and update UI immediately with available state
|
// Await Phase 1 calls (~40-190ms total) and update UI immediately with available state
|
||||||
|
|||||||
@@ -50,6 +50,7 @@ import com.m57.hermescontrol.data.remote.CleartextPolicy
|
|||||||
import com.m57.hermescontrol.data.remote.HermesApiService
|
import com.m57.hermescontrol.data.remote.HermesApiService
|
||||||
import com.m57.hermescontrol.data.remote.ServerEndpoint
|
import com.m57.hermescontrol.data.remote.ServerEndpoint
|
||||||
import com.m57.hermescontrol.data.ws.KanbanEventsClient
|
import com.m57.hermescontrol.data.ws.KanbanEventsClient
|
||||||
|
import com.m57.hermescontrol.data.ws.ModelOptionsRepository
|
||||||
import com.m57.hermescontrol.ui.channels.ChannelsViewModel
|
import com.m57.hermescontrol.ui.channels.ChannelsViewModel
|
||||||
import com.m57.hermescontrol.ui.connect.ConnectViewModel
|
import com.m57.hermescontrol.ui.connect.ConnectViewModel
|
||||||
import com.m57.hermescontrol.ui.cron.CronJobsViewModel
|
import com.m57.hermescontrol.ui.cron.CronJobsViewModel
|
||||||
@@ -1293,7 +1294,8 @@ class E2eIntegrationTest {
|
|||||||
),
|
),
|
||||||
)
|
)
|
||||||
|
|
||||||
val viewModel = ModelViewModel()
|
// Issue #1164: keep the real WS singleton off this test's mocked IO dispatcher.
|
||||||
|
val viewModel = ModelViewModel(ModelOptionsRepository(connected = { false }))
|
||||||
viewModel.loadAll()
|
viewModel.loadAll()
|
||||||
advanceUntilIdle()
|
advanceUntilIdle()
|
||||||
|
|
||||||
@@ -1352,7 +1354,8 @@ class E2eIntegrationTest {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Default loadAll() must call getModelOptions with refresh = false
|
// Default loadAll() must call getModelOptions with refresh = false
|
||||||
val viewModel = ModelViewModel()
|
// Issue #1164: keep the real WS singleton off this test's mocked IO dispatcher.
|
||||||
|
val viewModel = ModelViewModel(ModelOptionsRepository(connected = { false }))
|
||||||
viewModel.loadAll()
|
viewModel.loadAll()
|
||||||
advanceUntilIdle()
|
advanceUntilIdle()
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,96 @@
|
|||||||
|
package com.m57.hermescontrol.data.ws
|
||||||
|
|
||||||
|
import com.m57.hermescontrol.data.model.ModelOptionsResponse
|
||||||
|
import com.m57.hermescontrol.data.remote.NetworkResult
|
||||||
|
import kotlinx.coroutines.CompletableDeferred
|
||||||
|
import kotlinx.coroutines.async
|
||||||
|
import kotlinx.coroutines.awaitCancellation
|
||||||
|
import kotlinx.coroutines.test.runTest
|
||||||
|
import kotlinx.serialization.json.Json
|
||||||
|
import org.junit.Assert.assertEquals
|
||||||
|
import org.junit.Assert.assertTrue
|
||||||
|
import org.junit.Test
|
||||||
|
|
||||||
|
class ModelOptionsRepositoryTest {
|
||||||
|
private val fallback = NetworkResult.Success(ModelOptionsResponse(emptyList()))
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `connected uses raw JSON and preserves refresh flags without REST`() =
|
||||||
|
runTest {
|
||||||
|
val repository =
|
||||||
|
ModelOptionsRepository(
|
||||||
|
connected = { true },
|
||||||
|
request = { params ->
|
||||||
|
assertEquals(mapOf("refresh" to true, "include_unconfigured" to false), params)
|
||||||
|
Json.parseToJsonElement("""{"providers":[{"slug":"test","name":"Test","models":["a"]}]}""")
|
||||||
|
},
|
||||||
|
rest = { error("REST must not run") },
|
||||||
|
)
|
||||||
|
val result = repository.load(true) as NetworkResult.Success
|
||||||
|
assertEquals(
|
||||||
|
"test",
|
||||||
|
result.data.providers
|
||||||
|
.single()
|
||||||
|
.slug,
|
||||||
|
)
|
||||||
|
assertTrue(WsMethods.MODEL_OPTIONS in WsMethods.PROFILE_SCOPED_METHODS)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `disconnected bypasses RPC`() =
|
||||||
|
runTest {
|
||||||
|
val repository =
|
||||||
|
ModelOptionsRepository(
|
||||||
|
connected = { false },
|
||||||
|
request = { error("RPC must not run") },
|
||||||
|
rest = { refresh ->
|
||||||
|
assertTrue(refresh)
|
||||||
|
fallback
|
||||||
|
},
|
||||||
|
)
|
||||||
|
assertEquals(fallback, repository.load(true))
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `RPC failure and malformed envelopes use REST`() =
|
||||||
|
runTest {
|
||||||
|
for (wire in listOf(null, "{}", "[]", """{"providers":null}""")) {
|
||||||
|
var restCalls = 0
|
||||||
|
val repository =
|
||||||
|
ModelOptionsRepository(
|
||||||
|
connected = { true },
|
||||||
|
request = { if (wire == null) error("Unsupported method") else Json.parseToJsonElement(wire) },
|
||||||
|
rest = {
|
||||||
|
restCalls++
|
||||||
|
fallback
|
||||||
|
},
|
||||||
|
)
|
||||||
|
assertEquals(fallback, repository.load())
|
||||||
|
assertEquals(1, restCalls)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `cancelled screen never falls back`() =
|
||||||
|
runTest {
|
||||||
|
val started = CompletableDeferred<Unit>()
|
||||||
|
var restCalls = 0
|
||||||
|
val repository =
|
||||||
|
ModelOptionsRepository(
|
||||||
|
connected = { true },
|
||||||
|
request = {
|
||||||
|
started.complete(Unit)
|
||||||
|
awaitCancellation()
|
||||||
|
},
|
||||||
|
rest = {
|
||||||
|
restCalls++
|
||||||
|
fallback
|
||||||
|
},
|
||||||
|
)
|
||||||
|
val job = async { repository.load() }
|
||||||
|
started.await()
|
||||||
|
job.cancel()
|
||||||
|
job.join()
|
||||||
|
assertEquals(0, restCalls)
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in new issue
Block a user