Merge pull request #31 from OSGP/feature/FDP-2306

FDP-2306: handle failure urcs better

application/src/main/kotlin/org/gxf/crestdeviceservice/coap/PskErrorUrc.kt

@@ -0,0 +1,21 @@
+// SPDX-FileCopyrightText: Contributors to the GXF project
+//
+// SPDX-License-Identifier: Apache-2.0
+package org.gxf.crestdeviceservice.coap
+
+enum class PskErrorUrc(val code: String, val message: String) {
+    PSK_EQER("PSK:EQER", "Set PSK does not equal earlier PSK"),
+    PSK_DLNA("PSK:DLNA", "Downlink not allowed"),
+    PSK_DLER("PSK:DLER", "Downlink (syntax) error"),
+    PSK_HSER("PSK:HSER", "SHA256 hash error"),
+    PSK_CSER("PSK:CSER", "Checksum error");
+
+    companion object {
+        fun messageFromCode(code: String): String {
+            val error = entries.firstOrNull { it.code == code }
+            return error?.message ?: "Unknown URC"
+        }
+
+        fun isPskErrorURC(code: String) = entries.any { it.code == code }
+    }
+}

application/src/main/kotlin/org/gxf/crestdeviceservice/coap/UrcService.kt

@@ -12,9 +12,8 @@ import org.springframework.stereotype.Service
 @Service
 class UrcService(private val pskService: PskService) {
     companion object {
-        private const val URC_PSK_SUCCESS = "PSK:SET"
-        private const val URC_PSK_ERROR = "ER"
         private const val URC_FIELD = "URC"
+        private const val URC_PSK_SUCCESS = "PSK:SET"
     }
 
     private val logger = KotlinLogging.logger {}
@@ -29,34 +28,42 @@ class UrcService(private val pskService: PskService) {
         logger.debug { "Received message with urcs ${urcs.joinToString(", ")}" }
 
         when {
-            urcsContainsError(urcs) -> {
-                handleErrorUrc(identity)
+            urcsContainPskError(urcs) -> {
+                handlePskErrors(identity, urcs)
             }
-            urcsContainsSuccess(urcs) -> {
-                handleSuccessUrc(identity)
+            urcsContainPskSuccess(urcs) -> {
+                handlePskSuccess(identity)
             }
         }
     }
 
     private fun getUrcsFromMessage(body: JsonNode) =
         body[URC_FIELD].filter { it.isTextual }.map { it.asText() }
 
-    private fun urcsContainsError(urcs: List<String>) =
-        urcs.any { urc -> urc.contains(URC_PSK_ERROR) }
+    private fun urcsContainPskError(urcs: List<String>) =
+        urcs.any { urc -> PskErrorUrc.isPskErrorURC(urc) }
 
-    private fun handleErrorUrc(identity: String) {
+    private fun handlePskErrors(identity: String, urcs: List<String>) {
         if (!pskService.isPendingKeyPresent(identity)) {
             throw NoExistingPskException(
                 "Failure URC received, but no pending key present to set as invalid")
         }
-        logger.warn { "Error received for set PSK command, setting pending key to invalid" }
+
+        urcs
+            .filter { urc -> PskErrorUrc.isPskErrorURC(urc) }
+            .forEach { urc ->
+                logger.warn {
+                    "PSK set failed for device with id ${identity}: ${PskErrorUrc.messageFromCode(urc)}"
+                }
+            }
+
         pskService.setPendingKeyAsInvalid(identity)
     }
 
-    private fun urcsContainsSuccess(urcs: List<String>) =
+    private fun urcsContainPskSuccess(urcs: List<String>) =
         urcs.any { urc -> urc.contains(URC_PSK_SUCCESS) }
 
-    private fun handleSuccessUrc(identity: String) {
+    private fun handlePskSuccess(identity: String) {
         if (!pskService.isPendingKeyPresent(identity)) {
             throw NoExistingPskException(
                 "Success URC received, but no pending key present to set as active")

application/src/test/kotlin/org/gxf/crestdeviceservice/TestHelper.kt

@@ -3,11 +3,17 @@
 // SPDX-License-Identifier: Apache-2.0
 package org.gxf.crestdeviceservice
 
+import com.fasterxml.jackson.databind.ObjectMapper
+import com.fasterxml.jackson.databind.node.ObjectNode
 import java.time.Instant
 import org.gxf.crestdeviceservice.psk.entity.PreSharedKey
 import org.gxf.crestdeviceservice.psk.entity.PreSharedKeyStatus
+import org.mockito.kotlin.spy
+import org.springframework.util.ResourceUtils
 
 object TestHelper {
+    private val mapper = spy<ObjectMapper>()
+
     fun preSharedKeyReady() = preSharedKeyWithStatus(PreSharedKeyStatus.READY)
 
     fun preSharedKeyActive() = preSharedKeyWithStatus(PreSharedKeyStatus.ACTIVE)
@@ -16,4 +22,9 @@ object TestHelper {
 
     private fun preSharedKeyWithStatus(status: PreSharedKeyStatus) =
         PreSharedKey("identity", 1, Instant.now(), "key", "secret", status)
+
+    fun messageTemplate(): ObjectNode {
+        val messageFile = ResourceUtils.getFile("classpath:message-template.json")
+        return mapper.readTree(messageFile) as ObjectNode
+    }
 }

application/src/test/kotlin/org/gxf/crestdeviceservice/coap/DownlinkServiceTest.kt

@@ -3,39 +3,37 @@
 // SPDX-License-Identifier: Apache-2.0
 package org.gxf.crestdeviceservice.coap
 
-import com.fasterxml.jackson.databind.ObjectMapper
 import java.time.Instant
 import org.assertj.core.api.Assertions.assertThat
+import org.gxf.crestdeviceservice.TestHelper
 import org.gxf.crestdeviceservice.psk.PskService
 import org.gxf.crestdeviceservice.psk.entity.PreSharedKey
 import org.gxf.crestdeviceservice.psk.entity.PreSharedKeyStatus
 import org.junit.jupiter.api.Test
 import org.mockito.kotlin.mock
-import org.mockito.kotlin.spy
 import org.mockito.kotlin.whenever
-import org.springframework.util.ResourceUtils
 
 class DownlinkServiceTest {
     private val pskService = mock<PskService>()
     private val downLinkService = DownlinkService(pskService)
-    private val mapper = spy<ObjectMapper>()
+    private val message = TestHelper.messageTemplate()
+
+    companion object {
+        private const val IDENTITY = "867787050253370"
+    }
 
     @Test
     fun shouldReturnPskDownlinkWhenThereIsANewPsk() {
-        val identity = "identity"
         val expectedKey = "key"
         val expectedHash = "ad165b11320bc91501ab08613cc3a48a62a6caca4d5c8b14ca82cc313b3b96cd"
         val psk =
             PreSharedKey(
-                identity, 1, Instant.now(), expectedKey, "secret", PreSharedKeyStatus.PENDING)
+                IDENTITY, 1, Instant.now(), expectedKey, "secret", PreSharedKeyStatus.PENDING)
 
-        whenever(pskService.needsKeyChange(identity)).thenReturn(true)
-        whenever(pskService.setReadyKeyForIdentityAsPending(identity)).thenReturn(psk)
+        whenever(pskService.needsKeyChange(IDENTITY)).thenReturn(true)
+        whenever(pskService.setReadyKeyForIdentityAsPending(IDENTITY)).thenReturn(psk)
 
-        val fileToUse = ResourceUtils.getFile("classpath:messages/message.json")
-        val message = mapper.readTree(fileToUse)
-
-        val result = downLinkService.getDownlinkForIdentity(identity, message)
+        val result = downLinkService.getDownlinkForIdentity(IDENTITY, message)
 
         // Psk command is formatted as: PSK:[Key]:[Hash];PSK:[Key]:[Hash]:SET
         assertThat(result)
@@ -44,13 +42,9 @@ class DownlinkServiceTest {
 
     @Test
     fun shouldReturnNoActionDownlinkWhenThereIsNoNewPsk() {
-        val identity = "identity"
-        whenever(pskService.needsKeyChange(identity)).thenReturn(false)
-
-        val fileToUse = ResourceUtils.getFile("classpath:messages/message.json")
-        val message = mapper.readTree(fileToUse)
+        whenever(pskService.needsKeyChange(IDENTITY)).thenReturn(false)
 
-        val result = downLinkService.getDownlinkForIdentity(identity, message)
+        val result = downLinkService.getDownlinkForIdentity(IDENTITY, message)
 
         assertThat(result).isEqualTo("0")
     }

application/src/test/kotlin/org/gxf/crestdeviceservice/coap/UrcServiceTest.kt

@@ -3,46 +3,110 @@
 // SPDX-License-Identifier: Apache-2.0
 package org.gxf.crestdeviceservice.coap
 
+import com.fasterxml.jackson.databind.JsonNode
 import com.fasterxml.jackson.databind.ObjectMapper
+import com.fasterxml.jackson.databind.node.ArrayNode
+import com.fasterxml.jackson.databind.node.BaseJsonNode
+import com.fasterxml.jackson.databind.node.JsonNodeFactory
+import com.fasterxml.jackson.databind.node.ObjectNode
+import com.fasterxml.jackson.databind.node.TextNode
+import java.util.stream.Stream
+import org.gxf.crestdeviceservice.TestHelper
 import org.gxf.crestdeviceservice.psk.PskService
 import org.junit.jupiter.api.Test
+import org.junit.jupiter.params.ParameterizedTest
+import org.junit.jupiter.params.provider.MethodSource
 import org.mockito.kotlin.mock
 import org.mockito.kotlin.spy
+import org.mockito.kotlin.times
 import org.mockito.kotlin.verify
 import org.mockito.kotlin.whenever
-import org.springframework.util.ResourceUtils
 
 class UrcServiceTest {
     private val pskService = mock<PskService>()
     private val urcService = UrcService(pskService)
     private val mapper = spy<ObjectMapper>()
 
+    companion object {
+        private const val URC_FIELD = "URC"
+        private const val DL_FIELD = "DL"
+        private const val PSK_COMMAND =
+            "!PSK:umU6KJ4g7Ye5ZU6o:4a3cfdd487298e2f048ebfd703a1da4800c18f2167b62192cf7dc9fd6cc4bcd3;PSK:umU6KJ4g7Ye5ZU6o:4a3cfdd487298e2f048ebfd703a1da4800c18f2167b62192cf7dc9fd6cc4bcd3:SET"
+        private const val IDENTITY = "867787050253370"
+
+        @JvmStatic
+        private fun containingPskErrorUrcs() =
+            Stream.of(
+                listOf("PSK:EQER"),
+                listOf("PSK:DLNA"),
+                listOf("PSK:DLER"),
+                listOf("PSK:HSER"),
+                listOf("PSK:CSER"),
+                listOf("TS:ERR", "PSK:DLER"),
+                listOf("PSK:HSER", "PSK:DLER"))
+
+        @JvmStatic
+        private fun notContainingPskErrorUrcs() =
+            Stream.of(
+                listOf("INIT"),
+                listOf("ENPD"),
+                listOf("TEL:RBT"),
+                listOf("JTR"),
+                listOf("WDR"),
+                listOf("BOR"),
+                listOf("EXR"),
+                listOf("POR"),
+                listOf("TS:ERR"),
+                listOf("INIT", "BOR", "POR"),
+                listOf("OTA:HSER", "MSI:DLNA"))
+    }
+
     @Test
     fun shouldChangeActiveKeyWhenSuccessURCReceived() {
-        val identity = "identity"
+        val urcs = listOf("PSK:SET")
+        interpretURCWhileNewKeyIsPending(urcs)
+        verify(pskService).changeActiveKey(IDENTITY)
+    }
 
-        whenever(pskService.needsKeyChange(identity)).thenReturn(false)
-        whenever(pskService.isPendingKeyPresent(identity)).thenReturn(true)
+    @ParameterizedTest(name = "should set pending key as invalid for {0}")
+    @MethodSource("containingPskErrorUrcs")
+    fun shouldSetPendingKeyAsInvalidWhenFailureURCReceived(urcs: List<String>) {
+        interpretURCWhileNewKeyIsPending(urcs)
+        verify(pskService).setPendingKeyAsInvalid(IDENTITY)
+    }
 
-        val fileToUse = ResourceUtils.getFile("classpath:messages/message_psk_set_success.json")
-        val message = mapper.readTree(fileToUse)
+    @ParameterizedTest(name = "should not set pending key as invalid for {0}")
+    @MethodSource("notContainingPskErrorUrcs")
+    fun shouldNotSetPendingKeyAsInvalidWhenOtherURCReceived(urcs: List<String>) {
+        interpretURCWhileNewKeyIsPending(urcs)
+        verify(pskService, times(0)).setPendingKeyAsInvalid(IDENTITY)
+    }
 
-        urcService.interpretURCInMessage(identity, message)
+    private fun interpretURCWhileNewKeyIsPending(urcs: List<String>) {
+        whenever(pskService.needsKeyChange(IDENTITY)).thenReturn(false)
+        whenever(pskService.isPendingKeyPresent(IDENTITY)).thenReturn(true)
 
-        verify(pskService).changeActiveKey(identity)
-    }
+        val message = updatePskCommandInMessage(urcs)
 
-    @Test
-    fun shouldSetPendingKeyAsInvalidWhenFailureURCReceived() {
-        val identity = "identity"
+        urcService.interpretURCInMessage(IDENTITY, message)
+    }
 
-        whenever(pskService.needsKeyChange(identity)).thenReturn(false)
-        whenever(pskService.isPendingKeyPresent(identity)).thenReturn(true)
+    private fun updatePskCommandInMessage(urcs: List<String>): JsonNode {
+        val message = TestHelper.messageTemplate()
+        val urcFieldValue = urcFieldValue(urcs)
 
-        val fileToUse = ResourceUtils.getFile("classpath:messages/message_psk_set_failure.json")
-        val message = mapper.readTree(fileToUse)
-        urcService.interpretURCInMessage(identity, message)
+        message.replace(URC_FIELD, urcFieldValue)
+        return message
+    }
 
-        verify(pskService).setPendingKeyAsInvalid(identity)
+    private fun urcFieldValue(urcs: List<String>): ArrayNode? {
+        val urcNodes = urcs.map { urc -> TextNode(urc) }
+        val downlinkNode =
+            ObjectNode(JsonNodeFactory.instance, mapOf(DL_FIELD to TextNode(PSK_COMMAND)))
+        val urcsPlusReceivedDownlink: MutableList<BaseJsonNode> = mutableListOf()
+        urcsPlusReceivedDownlink.addAll(urcNodes)
+        urcsPlusReceivedDownlink.add(downlinkNode)
+        val urcFieldValue = mapper.valueToTree<ArrayNode>(urcsPlusReceivedDownlink)
+        return urcFieldValue
     }
 }