From 65a4d4f692008b39547e134de2d8d03fc315c763 Mon Sep 17 00:00:00 2001 From: James Rich Date: Thu, 21 May 2026 17:43:54 -0500 Subject: [PATCH] fix: propagate SendMessageUseCase errors and add provider/resolver tests - SendMessageUseCase now rethrows exceptions after logging (Finding #1) - AiFunctionProviderImpl catches send failures and returns InvalidArgument - Added AiFunctionProviderImplTest with 10 unit tests covering: - Disconnection checks for all three function groups - Node lookup (found, not found, null position, invalid hex) - Metrics aggregation (active nodes, empty, zero lastHeard, degraded health) - Rate limiting behavior - Expanded FuzzyNameResolverTest with 8 behavioral tests (Finding #5): - resolveNodeName: exact, fuzzy, ambiguous, not found - resolveChannelName: exact, admin exclusion, empty channels Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../core/data/ai/AiFunctionProviderImpl.kt | 17 +- .../data/ai/AiFunctionProviderImplTest.kt | 240 ++++++++++++++++++ .../core/data/ai/FuzzyNameResolverTest.kt | 142 +++++++++++ .../repository/usecase/SendMessageUseCase.kt | 1 + 4 files changed, 394 insertions(+), 6 deletions(-) create mode 100644 core/data/src/commonTest/kotlin/org/meshtastic/core/data/ai/AiFunctionProviderImplTest.kt diff --git a/core/data/src/commonMain/kotlin/org/meshtastic/core/data/ai/AiFunctionProviderImpl.kt b/core/data/src/commonMain/kotlin/org/meshtastic/core/data/ai/AiFunctionProviderImpl.kt index 352e3d1d6..ef3d572d6 100644 --- a/core/data/src/commonMain/kotlin/org/meshtastic/core/data/ai/AiFunctionProviderImpl.kt +++ b/core/data/src/commonMain/kotlin/org/meshtastic/core/data/ai/AiFunctionProviderImpl.kt @@ -85,13 +85,18 @@ class AiFunctionProviderImpl( val key = (contactKey as ResolvedContact.Resolved).contactKey // Send via existing use case and capture the generated messageId - val messageId = sendMessageUseCase.invoke(text, key) + try { + val messageId = sendMessageUseCase.invoke(text, key) - SendMessageResult.Success( - messageId = messageId, - channel = contactKey.channelName, - timestamp = clock.now().toEpochMilliseconds(), - ) + SendMessageResult.Success( + messageId = messageId, + channel = contactKey.channelName, + timestamp = clock.now().toEpochMilliseconds(), + ) + } catch (@Suppress("TooGenericExceptionCaught") ex: Exception) { + if (ex is CancellationException) throw ex + SendMessageResult.InvalidArgument("Failed to send message: ${ex.message}") + } } override suspend fun getMeshStatus(): MeshStatusResult = withTimeout(OPERATION_TIMEOUT) { diff --git a/core/data/src/commonTest/kotlin/org/meshtastic/core/data/ai/AiFunctionProviderImplTest.kt b/core/data/src/commonTest/kotlin/org/meshtastic/core/data/ai/AiFunctionProviderImplTest.kt new file mode 100644 index 000000000..11f1933ce --- /dev/null +++ b/core/data/src/commonTest/kotlin/org/meshtastic/core/data/ai/AiFunctionProviderImplTest.kt @@ -0,0 +1,240 @@ +/* + * Copyright (c) 2026 Meshtastic LLC + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.meshtastic.core.data.ai + +import dev.mokkery.MockMode +import dev.mokkery.answering.returns +import dev.mokkery.every +import dev.mokkery.mock +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.test.runTest +import org.meshtastic.core.model.ConnectionState +import org.meshtastic.core.model.Node +import org.meshtastic.core.repository.NodeRepository +import org.meshtastic.core.repository.RadioConfigRepository +import org.meshtastic.core.repository.ServiceRepository +import org.meshtastic.core.repository.usecase.SendMessageUseCase +import org.meshtastic.proto.User +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertIs +import kotlin.test.assertNull +import kotlin.time.Clock +import kotlin.time.Instant + +class AiFunctionProviderImplTest { + + private val connectionState = MutableStateFlow(ConnectionState.Connected) + private val serviceRepository: ServiceRepository = + mock(MockMode.autofill) { every { connectionState } returns this@AiFunctionProviderImplTest.connectionState } + private val nodeRepository: NodeRepository = mock(MockMode.autofill) + private val radioConfigRepository: RadioConfigRepository = mock(MockMode.autofill) + private val sendMessageUseCase: SendMessageUseCase = mock(MockMode.autofill) + private val fuzzyNameResolver = FuzzyNameResolver(nodeRepository, radioConfigRepository) + private val clock = TestClock(Instant.fromEpochSeconds(1_700_000_000)) + private val rateLimiter = RateLimiter(clock) + + private fun createProvider() = AiFunctionProviderImpl( + serviceRepository = serviceRepository, + nodeRepository = nodeRepository, + radioConfigRepository = radioConfigRepository, + sendMessageUseCase = sendMessageUseCase, + fuzzyNameResolver = fuzzyNameResolver, + rateLimiter = rateLimiter, + clock = clock, + ) + + // --- getNodeDetails tests --- + + @Test + fun getNodeDetails_returns_not_connected_when_disconnected() = runTest { + connectionState.value = ConnectionState.Disconnected + val provider = createProvider() + + val result = provider.getNodeDetails("!abc123") + assertIs(result) + } + + @Test + fun getNodeDetails_returns_not_found_for_unknown_node() = runTest { + val nodeMap = MutableStateFlow(emptyMap()) + every { nodeRepository.nodeDBbyNum } returns nodeMap + + val provider = createProvider() + val result = provider.getNodeDetails("!ffffff") + + assertIs(result) + } + + @Test + fun getNodeDetails_returns_node_data_for_valid_hex_id() = runTest { + val testNode = + Node( + num = 0xabc, + user = User(id = "!00000abc", long_name = "Alice", short_name = "AL"), + lastHeard = 1_700_000_000, + snr = 5.5f, + rssi = -70, + channel = 0, + hopsAway = 1, + ) + val nodeMap = MutableStateFlow(mapOf(0xabc to testNode)) + every { nodeRepository.nodeDBbyNum } returns nodeMap + + val provider = createProvider() + val result = provider.getNodeDetails("!abc") + + assertIs(result) + assertEquals("Alice", result.node.name) + assertEquals(5.5f, result.node.snr) + assertEquals(-70, result.node.rssi) + assertEquals(1, result.node.hopsAway) + } + + @Test + fun getNodeDetails_returns_null_position_when_no_fix() = runTest { + // Node with (0.0, 0.0) position and time=0 → no valid position + val testNode = Node(num = 1, user = User(id = "!00000001", long_name = "NoGPS", short_name = "NG")) + val nodeMap = MutableStateFlow(mapOf(1 to testNode)) + every { nodeRepository.nodeDBbyNum } returns nodeMap + + val provider = createProvider() + val result = provider.getNodeDetails("!1") + + assertIs(result) + assertNull(result.node.latitude) + assertNull(result.node.longitude) + } + + @Test + fun getNodeDetails_returns_error_for_invalid_hex_format() = runTest { + val nodeMap = MutableStateFlow(emptyMap()) + every { nodeRepository.nodeDBbyNum } returns nodeMap + + val provider = createProvider() + val result = provider.getNodeDetails("!not_hex") + + // Invalid hex should result in NotFound or Error + val isHandled = result is GetNodeDetailsResult.NotFound || result is GetNodeDetailsResult.Error + assertEquals(true, isHandled) + } + + // --- getMeshMetrics tests --- + + @Test + fun getMeshMetrics_returns_not_connected_when_disconnected() = runTest { + connectionState.value = ConnectionState.Disconnected + val provider = createProvider() + + val result = provider.getMeshMetrics() + assertIs(result) + } + + @Test + fun getMeshMetrics_returns_valid_metrics_with_active_nodes() = runTest { + val nodes = mapOf(1 to Node(num = 1, lastHeard = 1_699_999_990), 2 to Node(num = 2, lastHeard = 1_699_999_980)) + val nodeMap = MutableStateFlow(nodes) + every { nodeRepository.nodeDBbyNum } returns nodeMap + every { nodeRepository.totalNodeCount } returns flowOf(2) + every { nodeRepository.onlineNodeCount } returns flowOf(2) + + val provider = createProvider() + val result = provider.getMeshMetrics() + + assertIs(result) + assertEquals(2, result.metrics.totalNodeCount) + assertEquals(2, result.metrics.onlineNodeCount) + // Health score: 50 + (50 * 2) / 2 = 100 + assertEquals(100, result.metrics.meshHealthScore) + // Most recent packet: 1_699_999_990 * 1000 + assertEquals(1_699_999_990_000L, result.metrics.mostRecentPacketTime) + } + + @Test + fun getMeshMetrics_returns_zero_health_score_when_empty() = runTest { + val nodeMap = MutableStateFlow(emptyMap()) + every { nodeRepository.nodeDBbyNum } returns nodeMap + every { nodeRepository.totalNodeCount } returns flowOf(0) + every { nodeRepository.onlineNodeCount } returns flowOf(0) + + val provider = createProvider() + val result = provider.getMeshMetrics() + + assertIs(result) + assertEquals(0, result.metrics.totalNodeCount) + assertEquals(0, result.metrics.meshHealthScore) + } + + @Test + fun getMeshMetrics_falls_back_to_current_time_when_all_lastHeard_zero() = runTest { + val nodes = mapOf(1 to Node(num = 1, lastHeard = 0)) + val nodeMap = MutableStateFlow(nodes) + every { nodeRepository.nodeDBbyNum } returns nodeMap + every { nodeRepository.totalNodeCount } returns flowOf(1) + every { nodeRepository.onlineNodeCount } returns flowOf(0) + + val provider = createProvider() + val result = provider.getMeshMetrics() + + assertIs(result) + // Falls back to clock.now() since all lastHeard are 0 + assertEquals(clock.now().toEpochMilliseconds(), result.metrics.mostRecentPacketTime) + } + + @Test + fun getMeshMetrics_returns_degraded_health_when_no_nodes_online() = runTest { + val nodes = mapOf(1 to Node(num = 1, lastHeard = 1_000)) + val nodeMap = MutableStateFlow(nodes) + every { nodeRepository.nodeDBbyNum } returns nodeMap + every { nodeRepository.totalNodeCount } returns flowOf(1) + every { nodeRepository.onlineNodeCount } returns flowOf(0) + + val provider = createProvider() + val result = provider.getMeshMetrics() + + assertIs(result) + // HEALTH_SCORE_DEGRADED = 10 + assertEquals(10, result.metrics.meshHealthScore) + } + + // --- sendMessage error propagation test --- + + @Test + fun sendMessage_returns_not_connected_when_disconnected() = runTest { + connectionState.value = ConnectionState.Disconnected + val provider = createProvider() + + val result = provider.sendMessage("hello", null, null) + assertIs(result) + } + + @Test + fun sendMessage_returns_rate_limited_when_exhausted() = runTest { + val provider = createProvider() + + // Exhaust rate limit + repeat(RateLimiter.MAX_CALLS) { rateLimiter.tryAcquire() } + + val result = provider.sendMessage("hello", null, null) + assertIs(result) + } +} + +private class TestClock(var currentTime: Instant) : Clock { + override fun now(): Instant = currentTime +} diff --git a/core/data/src/commonTest/kotlin/org/meshtastic/core/data/ai/FuzzyNameResolverTest.kt b/core/data/src/commonTest/kotlin/org/meshtastic/core/data/ai/FuzzyNameResolverTest.kt index 3b4de1f79..3e75bfe74 100644 --- a/core/data/src/commonTest/kotlin/org/meshtastic/core/data/ai/FuzzyNameResolverTest.kt +++ b/core/data/src/commonTest/kotlin/org/meshtastic/core/data/ai/FuzzyNameResolverTest.kt @@ -16,6 +16,19 @@ */ package org.meshtastic.core.data.ai +import dev.mokkery.MockMode +import dev.mokkery.answering.returns +import dev.mokkery.every +import dev.mokkery.mock +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.test.runTest +import org.meshtastic.core.model.Node +import org.meshtastic.core.repository.NodeRepository +import org.meshtastic.core.repository.RadioConfigRepository +import org.meshtastic.proto.ChannelSet +import org.meshtastic.proto.ChannelSettings +import org.meshtastic.proto.User import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertIs @@ -79,4 +92,133 @@ class FuzzyNameResolverTest { assertEquals(1, result.channelIndex) assertEquals("General", result.name) } + + // --- Behavioral tests for resolveNodeName --- + + @Test + fun resolveNodeName_exact_match_case_insensitive() { + val nodeRepository: NodeRepository = mock(MockMode.autofill) + val radioConfigRepository: RadioConfigRepository = mock(MockMode.autofill) + val nodes = + mapOf( + 1 to Node(num = 1, user = User(id = "!00000001", long_name = "Alice", short_name = "AL")), + 2 to Node(num = 2, user = User(id = "!00000002", long_name = "Bob", short_name = "BO")), + ) + every { nodeRepository.nodeDBbyNum } returns MutableStateFlow(nodes) + + val resolver = FuzzyNameResolver(nodeRepository, radioConfigRepository) + val result = resolver.resolveNodeName("alice") + + assertIs(result) + assertEquals(1, result.nodeNum) + assertEquals("!00000001", result.userId) + } + + @Test + fun resolveNodeName_fuzzy_match_single_candidate() { + val nodeRepository: NodeRepository = mock(MockMode.autofill) + val radioConfigRepository: RadioConfigRepository = mock(MockMode.autofill) + val nodes = + mapOf( + 1 to Node(num = 1, user = User(id = "!00000001", long_name = "Alexander", short_name = "AX")), + 2 to Node(num = 2, user = User(id = "!00000002", long_name = "Bob", short_name = "BO")), + ) + every { nodeRepository.nodeDBbyNum } returns MutableStateFlow(nodes) + + val resolver = FuzzyNameResolver(nodeRepository, radioConfigRepository) + val result = resolver.resolveNodeName("Alexan") + + assertIs(result) + assertEquals(1, result.nodeNum) + } + + @Test + fun resolveNodeName_ambiguous_returns_candidates() { + val nodeRepository: NodeRepository = mock(MockMode.autofill) + val radioConfigRepository: RadioConfigRepository = mock(MockMode.autofill) + val nodes = + mapOf( + 1 to Node(num = 1, user = User(id = "!00000001", long_name = "Alice Smith", short_name = "AS")), + 2 to Node(num = 2, user = User(id = "!00000002", long_name = "Alice Jones", short_name = "AJ")), + ) + every { nodeRepository.nodeDBbyNum } returns MutableStateFlow(nodes) + + val resolver = FuzzyNameResolver(nodeRepository, radioConfigRepository) + val result = resolver.resolveNodeName("Alice") + + // "Alice" matches both equally via LCS + assertIs(result) + assertEquals(2, result.candidates.size) + } + + @Test + fun resolveNodeName_not_found_when_no_nodes() { + val nodeRepository: NodeRepository = mock(MockMode.autofill) + val radioConfigRepository: RadioConfigRepository = mock(MockMode.autofill) + every { nodeRepository.nodeDBbyNum } returns MutableStateFlow(emptyMap()) + + val resolver = FuzzyNameResolver(nodeRepository, radioConfigRepository) + val result = resolver.resolveNodeName("Unknown") + + assertIs(result) + } + + @Test + fun resolveNodeName_not_found_when_no_match() { + val nodeRepository: NodeRepository = mock(MockMode.autofill) + val radioConfigRepository: RadioConfigRepository = mock(MockMode.autofill) + val nodes = mapOf(1 to Node(num = 1, user = User(id = "!00000001", long_name = "Alice", short_name = "AL"))) + every { nodeRepository.nodeDBbyNum } returns MutableStateFlow(nodes) + + val resolver = FuzzyNameResolver(nodeRepository, radioConfigRepository) + val result = resolver.resolveNodeName("Zzzzzz") + + assertIs(result) + } + + // --- Behavioral tests for resolveChannelName --- + + @Test + fun resolveChannelName_exact_match() = runTest { + val nodeRepository: NodeRepository = mock(MockMode.autofill) + val radioConfigRepository: RadioConfigRepository = mock(MockMode.autofill) + val channelSet = + ChannelSet(settings = listOf(ChannelSettings(name = "General"), ChannelSettings(name = "Emergency"))) + every { radioConfigRepository.channelSetFlow } returns flowOf(channelSet) + + val resolver = FuzzyNameResolver(nodeRepository, radioConfigRepository) + val result = resolver.resolveChannelName("General") + + assertIs(result) + assertEquals(0, result.channelIndex) + assertEquals("General", result.name) + } + + @Test + fun resolveChannelName_excludes_admin_channel() = runTest { + val nodeRepository: NodeRepository = mock(MockMode.autofill) + val radioConfigRepository: RadioConfigRepository = mock(MockMode.autofill) + val channelSet = + ChannelSet(settings = listOf(ChannelSettings(name = "admin"), ChannelSettings(name = "General"))) + every { radioConfigRepository.channelSetFlow } returns flowOf(channelSet) + + val resolver = FuzzyNameResolver(nodeRepository, radioConfigRepository) + val result = resolver.resolveChannelName("admin") + + // "admin" should be excluded — cannot resolve to the admin channel + assertIs(result) + } + + @Test + fun resolveChannelName_not_found_when_empty() = runTest { + val nodeRepository: NodeRepository = mock(MockMode.autofill) + val radioConfigRepository: RadioConfigRepository = mock(MockMode.autofill) + val channelSet = ChannelSet(settings = emptyList()) + every { radioConfigRepository.channelSetFlow } returns flowOf(channelSet) + + val resolver = FuzzyNameResolver(nodeRepository, radioConfigRepository) + val result = resolver.resolveChannelName("General") + + assertIs(result) + } } diff --git a/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/usecase/SendMessageUseCase.kt b/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/usecase/SendMessageUseCase.kt index cf9969435..987c343ee 100644 --- a/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/usecase/SendMessageUseCase.kt +++ b/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/usecase/SendMessageUseCase.kt @@ -129,6 +129,7 @@ class SendMessageUseCaseImpl( messageQueue.enqueue(packetId) } catch (ex: Exception) { Logger.e(ex) { "Failed to enqueue message packet" } + throw ex } return packetId