From 4a93772b93017b1e0cb2ae985b32ac50af7a3be2 Mon Sep 17 00:00:00 2001 From: Hy4ri Date: Fri, 11 Sep 2026 18:00:47 +0300 Subject: [PATCH] refactor(mcp): extract AddServerRequestBuilder and McpImportSummary Extract request building and import summary formatting helpers. Wire into McpServersViewModel with unit test coverage. --- .../ui/mcp/AddServerRequestBuilder.kt | 56 +++++++++++ .../hermescontrol/ui/mcp/McpImportSummary.kt | 18 ++++ .../ui/mcp/McpServersViewModel.kt | 41 +------- .../ui/mcp/AddServerRequestBuilderTest.kt | 95 +++++++++++++++++++ .../ui/mcp/McpImportSummaryTest.kt | 24 +++++ 5 files changed, 195 insertions(+), 39 deletions(-) create mode 100644 app/src/main/java/com/m57/hermescontrol/ui/mcp/AddServerRequestBuilder.kt create mode 100644 app/src/main/java/com/m57/hermescontrol/ui/mcp/McpImportSummary.kt create mode 100644 app/src/test/java/com/m57/hermescontrol/ui/mcp/AddServerRequestBuilderTest.kt create mode 100644 app/src/test/java/com/m57/hermescontrol/ui/mcp/McpImportSummaryTest.kt diff --git a/app/src/main/java/com/m57/hermescontrol/ui/mcp/AddServerRequestBuilder.kt b/app/src/main/java/com/m57/hermescontrol/ui/mcp/AddServerRequestBuilder.kt new file mode 100644 index 00000000..4ab3070f --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/ui/mcp/AddServerRequestBuilder.kt @@ -0,0 +1,56 @@ +package com.m57.hermescontrol.ui.mcp + +import com.m57.hermescontrol.data.model.AddMcpServerRequest + +object AddServerRequestBuilder { + /** + * Constructs a validated AddMcpServerRequest from the add-server form parameters. + */ + fun build( + name: String, + mode: AddServerMode, + url: String, + command: String, + args: String, + auth: String, + bearerToken: String, + ): AddMcpServerRequest { + val isHttp = mode == AddServerMode.HTTP + val isStdio = mode == AddServerMode.Stdio + + val resolvedUrl = if (isHttp) url.trim().ifBlank { null } else null + val resolvedCommand = if (isStdio) command.trim().ifBlank { null } else null + val resolvedArgs = + if (isStdio && args.isNotBlank()) { + args.trim().split("\\s+".toRegex()).filter { it.isNotEmpty() } + } else { + null + } + val resolvedAuth = if (isHttp && auth != "none") auth else null + val resolvedBearerToken = + if (isHttp && auth == "header") bearerToken.trim().ifBlank { null } else null + + return AddMcpServerRequest( + name = name.trim(), + url = resolvedUrl, + command = resolvedCommand, + args = resolvedArgs, + auth = resolvedAuth, + bearerToken = resolvedBearerToken, + ) + } + + /** + * Overload reading form parameters directly from McpServersUiState. + */ + fun build(state: McpServersUiState): AddMcpServerRequest = + build( + name = state.addServerName, + mode = state.addMode, + url = state.addServerUrl, + command = state.addServerCommand, + args = state.addServerArgs, + auth = state.addServerAuth, + bearerToken = state.addServerBearerToken, + ) +} diff --git a/app/src/main/java/com/m57/hermescontrol/ui/mcp/McpImportSummary.kt b/app/src/main/java/com/m57/hermescontrol/ui/mcp/McpImportSummary.kt new file mode 100644 index 00000000..8155137e --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/ui/mcp/McpImportSummary.kt @@ -0,0 +1,18 @@ +package com.m57.hermescontrol.ui.mcp + +object McpImportSummary { + /** + * Formats the human-readable summary message reporting the result of a batch MCP JSON import. + */ + fun formatSummary( + successCount: Int, + errors: List, + ): String { + val errSummary = errors.joinToString("; ") + return when { + errors.isEmpty() -> "Successfully imported $successCount server(s)" + successCount > 0 -> "Imported $successCount server(s), ${errors.size} failed: $errSummary" + else -> "Failed to import: $errSummary" + } + } +} diff --git a/app/src/main/java/com/m57/hermescontrol/ui/mcp/McpServersViewModel.kt b/app/src/main/java/com/m57/hermescontrol/ui/mcp/McpServersViewModel.kt index 4d1d119e..4e9ead31 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/mcp/McpServersViewModel.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/mcp/McpServersViewModel.kt @@ -297,13 +297,7 @@ class McpServersViewModel : } _uiState.update { state -> - val errSummary = errors.joinToString("; ") - val msg = - when { - errors.isEmpty() -> "Successfully imported $successCount server(s)" - successCount > 0 -> "Imported $successCount server(s), ${errors.size} failed: $errSummary" - else -> "Failed to import: $errSummary" - } + val msg = McpImportSummary.formatSummary(successCount, errors) state.copy( isImportingJson = false, showImportDialog = false, @@ -357,38 +351,7 @@ class McpServersViewModel : } _uiState.update { it.copy(addingServer = true) } viewModelScope.launch { - val request = - AddMcpServerRequest( - name = state.addServerName.trim(), - url = if (state.addMode == AddServerMode.HTTP) state.addServerUrl.trim().ifBlank { null } else null, - command = - if (state.addMode == AddServerMode.Stdio) { - state.addServerCommand.trim().ifBlank { null } - } else { - null - }, - args = - if (state.addMode == AddServerMode.Stdio && state.addServerArgs.isNotBlank()) { - state.addServerArgs - .trim() - .split("\\s+".toRegex()) - .filter { it.isNotEmpty() } - } else { - null - }, - auth = - if (state.addMode == AddServerMode.HTTP && state.addServerAuth != "none") { - state.addServerAuth - } else { - null - }, - bearerToken = - if (state.addMode == AddServerMode.HTTP && state.addServerAuth == "header") { - state.addServerBearerToken.trim().ifBlank { null } - } else { - null - }, - ) + val request = AddServerRequestBuilder.build(state) val result = withContext(Dispatchers.IO) { safeApiCall { ApiClient.hermesApi.addMcpServer(request) } diff --git a/app/src/test/java/com/m57/hermescontrol/ui/mcp/AddServerRequestBuilderTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/mcp/AddServerRequestBuilderTest.kt new file mode 100644 index 00000000..36094914 --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/ui/mcp/AddServerRequestBuilderTest.kt @@ -0,0 +1,95 @@ +package com.m57.hermescontrol.ui.mcp + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +class AddServerRequestBuilderTest { + @Test + fun `build HTTP mode creates request with url and auth`() { + val request = + AddServerRequestBuilder.build( + name = " weather-server ", + mode = AddServerMode.HTTP, + url = " https://api.weather.com/mcp ", + command = "ignored-cmd", + args = "ignored-arg", + auth = "header", + bearerToken = " secret-token ", + ) + assertEquals("weather-server", request.name) + assertEquals("https://api.weather.com/mcp", request.url) + assertNull(request.command) + assertNull(request.args) + assertEquals("header", request.auth) + assertEquals("secret-token", request.bearerToken) + } + + @Test + fun `build HTTP mode with none auth nulls auth and bearerToken`() { + val request = + AddServerRequestBuilder.build( + name = "public-server", + mode = AddServerMode.HTTP, + url = "https://example.com/mcp", + command = "", + args = "", + auth = "none", + bearerToken = "should-be-ignored", + ) + assertEquals("public-server", request.name) + assertEquals("https://example.com/mcp", request.url) + assertNull(request.auth) + assertNull(request.bearerToken) + } + + @Test + fun `build Stdio mode creates request with command and parsed args`() { + val request = + AddServerRequestBuilder.build( + name = "local-tools", + mode = AddServerMode.Stdio, + url = "https://ignored.com", + command = " python3 ", + args = " -m mcp_server --debug ", + auth = "header", + bearerToken = "token", + ) + assertEquals("local-tools", request.name) + assertNull(request.url) + assertEquals("python3", request.command) + assertEquals(listOf("-m", "mcp_server", "--debug"), request.args) + assertNull(request.auth) + assertNull(request.bearerToken) + } + + @Test + fun `build Stdio mode with empty args nulls args`() { + val request = + AddServerRequestBuilder.build( + name = "npx-server", + mode = AddServerMode.Stdio, + url = "", + command = "npx server", + args = " ", + auth = "none", + bearerToken = "", + ) + assertEquals("npx server", request.command) + assertNull(request.args) + } + + @Test + fun `build from McpServersUiState populates fields correctly`() { + val state = + McpServersUiState( + addServerName = "state-server", + addMode = AddServerMode.HTTP, + addServerUrl = "https://state.com", + addServerAuth = "none", + ) + val request = AddServerRequestBuilder.build(state) + assertEquals("state-server", request.name) + assertEquals("https://state.com", request.url) + } +} diff --git a/app/src/test/java/com/m57/hermescontrol/ui/mcp/McpImportSummaryTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/mcp/McpImportSummaryTest.kt new file mode 100644 index 00000000..d0ba2738 --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/ui/mcp/McpImportSummaryTest.kt @@ -0,0 +1,24 @@ +package com.m57.hermescontrol.ui.mcp + +import org.junit.Assert.assertEquals +import org.junit.Test + +class McpImportSummaryTest { + @Test + fun `formatSummary returns success message when zero errors`() { + val result = McpImportSummary.formatSummary(3, emptyList()) + assertEquals("Successfully imported 3 server(s)", result) + } + + @Test + fun `formatSummary returns partial success message when some failed`() { + val result = McpImportSummary.formatSummary(2, listOf("serverA: timeout", "serverB: 404")) + assertEquals("Imported 2 server(s), 2 failed: serverA: timeout; serverB: 404", result) + } + + @Test + fun `formatSummary returns failure message when all failed`() { + val result = McpImportSummary.formatSummary(0, listOf("serverA: connection refused")) + assertEquals("Failed to import: serverA: connection refused", result) + } +}