Fix issue caused by reference type in jsonRpc_handleReply and optimiz…

…e jsonRpc_prepareInvokeRequest.

1. Remove unnecessary argument list iteration by removing usages of dynFunction_argumentTypeForIndex and dynFunction_argumentMetaForIndex.
2. Add more tests using error injection.

libs/dfi/gtest/CMakeLists.txt

@@ -46,6 +46,7 @@ if (EI_TESTS)
 			src/dyn_message_ei_tests.cc
 			src/dyn_function_ei_tests.cc
 			src/json_serializer_ei_tests.cc
+			src/json_rpc_ei_tests.cc
 	)
 	target_link_libraries(test_dfi_with_ei PRIVATE
 			dfi_cut

libs/dfi/gtest/src/json_rpc_ei_tests.cc

@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "json_rpc.h"
+#include "dyn_function.h"
+#include "celix_err.h"
+#include "jansson_ei.h"
+
+#include <gtest/gtest.h>
+
+class JsonRpcErrorInjectionTestSuite : public ::testing::Test {
+public:
+    JsonRpcErrorInjectionTestSuite() {
+
+    }
+    ~JsonRpcErrorInjectionTestSuite() override {
+        celix_ei_expect_json_dumps(nullptr, 0, nullptr);
+        celix_ei_expect_json_array_append_new(nullptr, 0, -1);
+        celix_ei_expect_json_string(nullptr, 0, nullptr);
+        celix_ei_expect_json_array(nullptr, 0, nullptr);
+        celix_err_resetErrors();
+    }
+};
+
+TEST_F(JsonRpcErrorInjectionTestSuite, prepareErrorTest) {
+    dyn_function_type *dynFunc = nullptr;
+    int rc = dynFunction_parseWithStr("add(#am=handle;PDD#am=pre;*D)N", nullptr, &dynFunc);
+    ASSERT_EQ(0, rc);
+
+    char *result = nullptr;
+
+    void *handle = nullptr;
+    double arg1 = 1.0;
+    double arg2 = 2.0;
+
+    void *args[4];
+    args[0] = &handle;
+    args[1] = &arg1;
+    args[2] = &arg2;
+
+    celix_ei_expect_json_array((void*)jsonRpc_prepareInvokeRequest, 0, nullptr);
+    rc = jsonRpc_prepareInvokeRequest(dynFunc, "add", args, &result);
+    ASSERT_NE(0, rc);
+    EXPECT_STREQ("Error adding arguments array for 'add'", celix_err_popLastError());
+
+    celix_ei_expect_json_string((void*)jsonRpc_prepareInvokeRequest, 0, nullptr);
+    rc = jsonRpc_prepareInvokeRequest(dynFunc, "add", args, &result);
+    ASSERT_NE(0, rc);
+    EXPECT_STREQ("Error setting method name 'add'", celix_err_popLastError());
+
+    celix_ei_expect_json_array_append_new((void*)jsonRpc_prepareInvokeRequest, 0, -1);
+    rc = jsonRpc_prepareInvokeRequest(dynFunc, "add", args, &result);
+    ASSERT_NE(0, rc);
+    EXPECT_STREQ("Error adding argument (1) for 'add'", celix_err_popLastError());
+
+    celix_ei_expect_json_dumps((void*)jsonRpc_prepareInvokeRequest, 0, nullptr);
+    rc = jsonRpc_prepareInvokeRequest(dynFunc, "add", args, &result);
+    ASSERT_NE(0, rc);
+
+    dynFunction_destroy(dynFunc);
+}

libs/dfi/src/json_rpc.c

@@ -219,58 +219,53 @@ int jsonRpc_call(const dyn_interface_type* intf, void* service, const char* requ
 }
 
 int jsonRpc_prepareInvokeRequest(const dyn_function_type* func, const char* id, void* args[], char** out) {
-    int status = OK;
-
-
-    json_t* invoke = json_object();
-    json_object_set_new_nocheck(invoke, "m", json_string(id));
+    json_auto_t* invoke = json_object();
+    // each method must have a non-null id
+    if (json_object_set_new_nocheck(invoke, "m", json_string(id)) != 0) {
+        celix_err_pushf("Error setting method name '%s'", id);
+        return ERROR;
+    }
 
-    json_t* arguments = json_array();
-    json_object_set_new_nocheck(invoke, "a", arguments);
+    json_auto_t* arguments = json_array();
+    if (json_object_set_nocheck(invoke, "a", arguments) != 0) {
+        celix_err_pushf("Error adding arguments array for '%s'", id);
+        return ERROR;
+    }
 
-    int i;
-    int nrOfArgs = dynFunction_nrOfArguments(func);
-    for (i = 0; i < nrOfArgs; i +=1) {
-        const dyn_type* type = dynFunction_argumentTypeForIndex(func, i);
-        enum dyn_function_argument_meta  meta = dynFunction_argumentMetaForIndex(func, i);
+    const struct dyn_function_arguments_head* dynArgs = dynFunction_arguments(func);
+    dyn_function_argument_type* entry = NULL;
+    TAILQ_FOREACH(entry, dynArgs, entries) {
+        const dyn_type* type = dynType_realType(entry->type);
+        enum dyn_function_argument_meta meta = entry->argumentMeta;
         if (meta == DYN_FUNCTION_ARGUMENT_META__STD) {
             json_t* val = NULL;
 
-            int rc = jsonSerializer_serializeJson(type, args[i], &val);
+            int rc = jsonSerializer_serializeJson(type, args[entry->index], &val);
+            if (rc != 0) {
+                celix_err_pushf("Failed to serialize args for function '%s'\n", id);
+                return ERROR;
+            }
 
             if (dynType_descriptorType(type) == 't') {
                 const char* metaArgument = dynType_getMetaInfo(type, "const");
-                if (metaArgument != NULL && strncmp("true", metaArgument, 5) == 0) {
+                if (metaArgument != NULL && strcmp("true", metaArgument) == 0) {
                     //const char * as input -> nop
                 } else {
-                    char** str = args[i];
+                    char** str = args[entry->index];
                     free(*str); //char * as input -> got ownership -> free it.
                 }
             }
-
-            if (rc == 0) {
-                json_array_append_new(arguments, val);
-            } else {
-                celix_err_pushf("Failed to serialize args for function '%s'\n", id);
-                status = ERROR;
-                break;
+            if (json_array_append_new(arguments, val) != 0) {
+                celix_err_pushf("Error adding argument (%d) for '%s'", entry->index, id);
+                return ERROR;
             }
-        } else {
-            //skip handle / output types
         }
     }
 
-    char* invokeStr = json_dumps(invoke, JSON_COMPACT | JSON_ENCODE_ANY);//Should use JSON_COMPACT, it can reduce the size of the JSON string.
-    json_decref(invoke);
-
-    if (status == OK) {
-        *out = invokeStr;
-    } else {
-        *out = NULL;
-        free(invokeStr);
-    }
-
-    return status;
+    //use JSON_COMPACT to reduce the size of the JSON string.
+    char* invokeStr = json_dumps(invoke, JSON_COMPACT | JSON_ENCODE_ANY);
+    *out = invokeStr;
+    return *out != NULL ? OK : ERROR;
 }
 
 int jsonRpc_handleReply(const dyn_function_type* func, const char* reply, void* args[], int* rsErrno) {
@@ -289,10 +284,10 @@ int jsonRpc_handleReply(const dyn_function_type* func, const char* reply, void*
 
     const struct dyn_function_arguments_head* arguments = dynFunction_arguments(func);
     dyn_function_argument_type* last = TAILQ_LAST(arguments, dyn_function_arguments_head);
-    const dyn_type* argType = last->type;
+    const dyn_type* argType = dynType_realType(last->type);
     enum dyn_function_argument_meta meta = last->argumentMeta;
     rsError = json_object_get(replyJson, "e");
-    if(rsError != NULL) {
+    if (rsError != NULL) {
         //get the invocation error of remote service function
         *rsErrno = (int) json_integer_value(rsError);
         return OK;
@@ -315,12 +310,12 @@ int jsonRpc_handleReply(const dyn_function_type* func, const char* reply, void*
         void** out = lastArg;
         size_t size = 0;
 
-        argType = dynType_typedPointer_getTypedType(argType);
-        status = jsonSerializer_deserializeJson(argType, result, &tmp);
+        const dyn_type* subType = dynType_typedPointer_getTypedType(argType);
+        status = jsonSerializer_deserializeJson(subType, result, &tmp);
         if (tmp != NULL) {
-            size = dynType_size(argType);
+            size = dynType_size(subType);
             memcpy(*out, tmp, size);
-            dynType_free(argType, tmp);
+            dynType_free(subType, tmp);
         }
     } else {
         const dyn_type* subType = dynType_typedPointer_getTypedType(argType);

libs/error_injector/jansson/CMakeLists.txt

@@ -33,5 +33,6 @@ target_link_options(jansson_ei INTERFACE
         LINKER:--wrap,json_array
         LINKER:--wrap,json_array_append_new
         LINKER:--wrap,json_integer
+        LINKER:--wrap,json_string
         )
 add_library(Celix::jansson_ei ALIAS jansson_ei)

libs/error_injector/jansson/include/jansson_ei.h

@@ -32,6 +32,7 @@ CELIX_EI_DECLARE(json_object_set_new, int);
 CELIX_EI_DECLARE(json_array, json_t*);
 CELIX_EI_DECLARE(json_array_append_new, int);
 CELIX_EI_DECLARE(json_integer, json_t*);
+CELIX_EI_DECLARE(json_string, json_t*);
 
 #ifdef __cplusplus
 }

libs/error_injector/jansson/src/jansson_ei.cc

@@ -74,4 +74,10 @@ json_t* __wrap_json_integer(json_int_t value) {
     return __real_json_integer(value);
 }
 
+json_t* __real_json_string(const char *value);
+CELIX_EI_DEFINE(json_string, json_t*)
+json_t* __wrap_json_string(const char *value) {
+    CELIX_EI_IMPL(json_string);
+    return __real_json_string(value);
+}
 }