Bugfix: Correct handling of communication status in InMemoryRpcClient

Previously, the InMemoryRpcClient raised a UStatusException if the message had any communication status without checking its value. According to the up-spec, a communication status value of 0 or no value indicates no communication error. The exception should only be raised if the communication status value is non-zero. This commit updates the logic to check the communication status value before raising the exception.

src/main/java/org/eclipse/uprotocol/communication/InMemoryRpcClient.java

@@ -137,7 +137,7 @@ private void handleResponses(UMessage response) {
         }
 
         // Check if the response has a commstatus and if it is not OK then complete the future with an exception
-        if (responseAttributes.hasCommstatus()) {
+        if (responseAttributes.hasCommstatus() && responseAttributes.getCommstatus() != UCode.OK) {
             final UCode code = responseAttributes.getCommstatus();
             responseFuture.completeExceptionally(
                 new UStatusException(code, "Communication error [" + code + "]"));

src/test/java/org/eclipse/uprotocol/communication/InMemoryRpcClientTest.java

@@ -19,6 +19,7 @@
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
+import java.util.Optional;
 import java.util.concurrent.CompletionStage;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.CompletableFuture;
@@ -194,6 +195,22 @@ public UMessage buildResponse(UMessage request) {
         assertTrue(response.toCompletableFuture().isCompletedExceptionally());
     }
 
+    @Test
+    @DisplayName("Test calling invokeMethod when we set comm status to UCode.OK")
+    public void testInvokeMethodWithCommStatusUCodeOKTransport() {
+        RpcClient rpcClient = new InMemoryRpcClient(new CommStatusOkTransport());
+        UPayload payload = UPayload.packToAny(UUri.newBuilder().build());
+        CompletionStage<UPayload> response = rpcClient.invokeMethod(createMethodUri(), payload, null);
+        assertFalse(response.toCompletableFuture().isCompletedExceptionally());
+        assertDoesNotThrow(() -> {
+            Optional<UStatus> unpackedStatus = UPayload.unpack(response.toCompletableFuture().get(), UStatus.class);
+            assertTrue(unpackedStatus.isPresent());
+            assertEquals(UCode.OK, unpackedStatus.get().getCode());
+            assertEquals("No Communication Error", unpackedStatus.get().getMessage());
+        });
+    }
+
+
 
     private UUri createMethodUri() {
         return UUri.newBuilder()

src/test/java/org/eclipse/uprotocol/communication/TestUTransport.java

@@ -168,6 +168,21 @@ public UMessage buildResponse(UMessage request) {
     }
 };
 
+/**
+ * Test UTransport that will set the commstatus for a success response
+ */
+class CommStatusOkTransport extends TestUTransport {
+    @Override
+    public UMessage buildResponse(UMessage request) {
+        UStatus status = UStatus.newBuilder()
+                .setCode(UCode.OK)
+                .setMessage("No Communication Error")
+                .build();
+        return UMessageBuilder.response(request.getAttributes())
+                .withCommStatus(status.getCode())
+                .build(UPayload.pack(status));
+    }
+}
 
 class EchoUTransport extends TestUTransport {
     @Override