From fb5beed35ff5f4a042aef2af19784ad059ba6833 Mon Sep 17 00:00:00 2001
From: Neelam Kushwah <neelam.kushwah@gm.com>
Date: Tue, 23 Jul 2024 21:42:19 -0400
Subject: [PATCH] Bugfix: Correct handling of communication status in
 InMemoryRpcClient (#163)

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.
---
 .../communication/InMemoryRpcClient.java        |  2 +-
 .../communication/InMemoryRpcClientTest.java    | 17 +++++++++++++++++
 .../uprotocol/communication/TestUTransport.java | 15 +++++++++++++++
 3 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/src/main/java/org/eclipse/uprotocol/communication/InMemoryRpcClient.java b/src/main/java/org/eclipse/uprotocol/communication/InMemoryRpcClient.java
index 7e691443..e9bb4908 100644
--- a/src/main/java/org/eclipse/uprotocol/communication/InMemoryRpcClient.java
+++ b/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 + "]"));
diff --git a/src/test/java/org/eclipse/uprotocol/communication/InMemoryRpcClientTest.java b/src/test/java/org/eclipse/uprotocol/communication/InMemoryRpcClientTest.java
index 16f69300..97511334 100644
--- a/src/test/java/org/eclipse/uprotocol/communication/InMemoryRpcClientTest.java
+++ b/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()
diff --git a/src/test/java/org/eclipse/uprotocol/communication/TestUTransport.java b/src/test/java/org/eclipse/uprotocol/communication/TestUTransport.java
index e70d2dad..67a7efe2 100644
--- a/src/test/java/org/eclipse/uprotocol/communication/TestUTransport.java
+++ b/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