Merge pull request #400 from xuzhenbao/rs_error_code_fix

Improved error codes for remote services

bundles/remote_services/remote_service_admin_dfi/gtest/CMakeLists.txt

@@ -57,6 +57,8 @@ get_property(remote_example_bundle_file TARGET remote_example_service PROPERTY B
 configure_file(config.properties.in config.properties)
 configure_file(client.properties.in client.properties)
 configure_file(server.properties.in server.properties)
+#add exception service interface descriptor
+configure_file(exception_test_service.descriptor exception_test_service.descriptor)
 
 add_dependencies(test_rsa_dfi
         rsa_dfi_bundle #note depend on the target creating the bundle zip not the lib target

bundles/remote_services/remote_service_admin_dfi/gtest/exception_test_service.descriptor

@@ -0,0 +1,9 @@
+:header
+type=interface
+name=exception_test
+version=1.0.0
+:annotations
+classname=exception_test_service
+:types
+:methods
+func1(V)V=func1(#am=handle;P)N

bundles/remote_services/remote_service_admin_dfi/gtest/src/rsa_client_server_tests.cc

@@ -34,13 +34,29 @@ extern "C" {
 #include "celix_launcher.h"
 #include "framework.h"
 #include "remote_service_admin.h"
+#include "remote_interceptor.h"
+
+#define RSA_DIF_EXCEPTION_TEST_SERVICE "exception_test_service"
+typedef struct rsa_dfi_exception_test_service {
+    void *handle;
+    int (*func1)(void *handle);
+}rsa_dfi_exception_test_service_t;
 
     static celix_framework_t *serverFramework = NULL;
     static celix_bundle_context_t *serverContext = NULL;
 
     static celix_framework_t *clientFramework = NULL;
     static celix_bundle_context_t *clientContext = NULL;
 
+    static rsa_dfi_exception_test_service_t *exceptionTestService = NULL;
+    static long exceptionTestSvcId = -1L;
+    static remote_interceptor_t *serverSvcInterceptor=NULL;
+    static remote_interceptor_t *clientSvcInterceptor=NULL;
+    static long serverSvcInterceptorSvcId = -1L;
+    static long clientSvcInterceptorSvcId = -1L;
+    static bool clientInterceptorPreProxyCallRetval=true;
+    static bool svcInterceptorPreExportCallRetval=true;
+
     static void setupFm(bool useCurlShare) {
         //server
         celix_properties_t *serverProps = celix_properties_load("server.properties");
@@ -65,6 +81,99 @@ extern "C" {
         celix_frameworkFactory_destroyFramework(clientFramework);
     }
 
+    static int rsaDfi_excepTestFunc1(void *handle __attribute__((unused))) {
+        return CELIX_CUSTOMER_ERROR_MAKE(0,1);
+    }
+
+    static void registerExceptionTestServer(void) {
+        celix_properties_t *properties = celix_properties_create();
+        celix_properties_set(properties, OSGI_RSA_SERVICE_EXPORTED_INTERFACES, RSA_DIF_EXCEPTION_TEST_SERVICE);
+        celix_properties_set(properties, OSGI_RSA_SERVICE_EXPORTED_CONFIGS, "org.amdatu.remote.admin.http");
+        exceptionTestService = (rsa_dfi_exception_test_service_t *)calloc(1,sizeof(*exceptionTestService));
+        exceptionTestService->handle = NULL;
+        exceptionTestService->func1 = rsaDfi_excepTestFunc1;
+        exceptionTestSvcId = celix_bundleContext_registerService(serverContext, exceptionTestService, RSA_DIF_EXCEPTION_TEST_SERVICE, properties);
+    }
+
+    static void unregisterExceptionTestServer(void) {
+        celix_bundleContext_unregisterService(serverContext, exceptionTestSvcId);
+        free(exceptionTestService);
+    }
+
+    static bool serverServiceInterceptor_preProxyCall(void *, const celix_properties_t *, const char *, celix_properties_t *) {
+        return true;
+    }
+
+    static void serverServiceInterceptor_postProxyCall(void *, const celix_properties_t *, const char *, celix_properties_t *) {
+
+    }
+
+    static bool serverServiceInterceptor_preExportCall(void *, const celix_properties_t *, const char *, celix_properties_t *) {
+        return svcInterceptorPreExportCallRetval;
+    }
+
+    static void serverServiceInterceptor_postExportCall(void *, const celix_properties_t *, const char *, celix_properties_t *) {
+
+    }
+
+    static bool clientServiceInterceptor_preProxyCall(void *, const celix_properties_t *, const char *, celix_properties_t *) {
+        return clientInterceptorPreProxyCallRetval;
+    }
+
+    static void clientServiceInterceptor_postProxyCall(void *, const celix_properties_t *, const char *, celix_properties_t *) {
+
+    }
+
+    static bool clientServiceInterceptor_preExportCall(void *, const celix_properties_t *, const char *, celix_properties_t *) {
+        return true;
+    }
+
+    static void clientServiceInterceptor_postExportCall(void *, const celix_properties_t *, const char *, celix_properties_t *) {
+
+    }
+
+    static void registerInterceptorService(void) {
+        svcInterceptorPreExportCallRetval = true;
+        serverSvcInterceptor = (remote_interceptor_t *)calloc(1,sizeof(*serverSvcInterceptor));
+        serverSvcInterceptor->handle = NULL;
+        serverSvcInterceptor->preProxyCall = serverServiceInterceptor_preProxyCall;
+        serverSvcInterceptor->postProxyCall = serverServiceInterceptor_postProxyCall;
+        serverSvcInterceptor->preExportCall = serverServiceInterceptor_preExportCall;
+        serverSvcInterceptor->postExportCall = serverServiceInterceptor_postExportCall;
+        celix_properties_t *svcInterceptorProps = celix_properties_create();
+        celix_properties_setLong(svcInterceptorProps, OSGI_FRAMEWORK_SERVICE_RANKING, 10);
+        celix_service_registration_options_t svcInterceptorOpts{};
+        svcInterceptorOpts.svc = serverSvcInterceptor;
+        svcInterceptorOpts.serviceName = REMOTE_INTERCEPTOR_SERVICE_NAME;
+        svcInterceptorOpts.serviceVersion = REMOTE_INTERCEPTOR_SERVICE_VERSION;
+        svcInterceptorOpts.properties = svcInterceptorProps;
+        serverSvcInterceptorSvcId = celix_bundleContext_registerServiceWithOptions(serverContext, &svcInterceptorOpts);
+
+        clientInterceptorPreProxyCallRetval = true;
+        clientSvcInterceptor = (remote_interceptor_t *)calloc(1,sizeof(*clientSvcInterceptor));
+        clientSvcInterceptor->handle = NULL;
+        clientSvcInterceptor->preProxyCall = clientServiceInterceptor_preProxyCall;
+        clientSvcInterceptor->postProxyCall = clientServiceInterceptor_postProxyCall;
+        clientSvcInterceptor->preExportCall = clientServiceInterceptor_preExportCall;
+        clientSvcInterceptor->postExportCall = clientServiceInterceptor_postExportCall;
+        celix_properties_t *clientInterceptorProps = celix_properties_create();
+        celix_properties_setLong(clientInterceptorProps, OSGI_FRAMEWORK_SERVICE_RANKING, 10);
+        celix_service_registration_options_t clientInterceptorOpts{};
+        clientInterceptorOpts.svc = clientSvcInterceptor;
+        clientInterceptorOpts.serviceName = REMOTE_INTERCEPTOR_SERVICE_NAME;
+        clientInterceptorOpts.serviceVersion = REMOTE_INTERCEPTOR_SERVICE_VERSION;
+        clientInterceptorOpts.properties = clientInterceptorProps;
+        clientSvcInterceptorSvcId = celix_bundleContext_registerServiceWithOptions(clientContext, &clientInterceptorOpts);
+    }
+
+    static void unregisterInterceptorService(void) {
+        celix_bundleContext_unregisterService(clientContext, clientSvcInterceptorSvcId);
+        free(clientSvcInterceptor);
+
+        celix_bundleContext_unregisterService(serverContext, serverSvcInterceptorSvcId);
+        free(serverSvcInterceptor);
+    }
+
     static void testComplex(void *handle __attribute__((unused)), void *svc) {
         auto *tst = static_cast<tst_service_t *>(svc);
 
@@ -149,6 +258,37 @@ extern "C" {
         ASSERT_TRUE(ok);
     };
 
+    static void testInterceptorPreExportCallReturnFalse(void *handle __attribute__((unused)), void *svc) {
+        svcInterceptorPreExportCallRetval = false;
+        auto *tst = static_cast<tst_service_t *>(svc);
+
+        bool ok = tst->testRemoteAction(tst->handle);
+        ASSERT_FALSE(ok);
+    }
+
+    static void testInterceptorPreProxyCallReturnFalse(void *handle __attribute__((unused)), void *svc) {
+        clientInterceptorPreProxyCallRetval = false;
+        auto *tst = static_cast<tst_service_t *>(svc);
+
+        bool ok = tst->testRemoteAction(tst->handle);
+        ASSERT_FALSE(ok);
+    }
+
+    static void testExceptionServiceCallback(void *handle __attribute__((unused)), void *svc) {
+        rsa_dfi_exception_test_service_t * service = (rsa_dfi_exception_test_service_t *)(svc);
+        int ret = service->func1(service->handle);
+        EXPECT_EQ(CELIX_CUSTOMER_ERROR_MAKE(0,1),ret);
+    }
+
+    static void testExceptionService(void) {
+        celix_service_use_options_t opts{};
+        opts.filter.serviceName = RSA_DIF_EXCEPTION_TEST_SERVICE;
+        opts.use = testExceptionServiceCallback;
+        opts.filter.ignoreServiceLanguage = true;
+        opts.waitTimeoutInSeconds = 2;
+        bool called = celix_bundleContext_useServiceWithOptions(clientContext, &opts);
+        ASSERT_TRUE(called);
+    }
 }
 
 template<typename F>
@@ -184,6 +324,29 @@ class RsaDfiClientServerWithCurlShareTests : public ::testing::Test {
 
 };
 
+class RsaDfiClientServerInterceptorTests : public ::testing::Test {
+public:
+    RsaDfiClientServerInterceptorTests() {
+        setupFm(false);
+        registerInterceptorService();
+    }
+    ~RsaDfiClientServerInterceptorTests() override {
+        unregisterInterceptorService();
+        teardownFm();
+    }
+};
+
+class RsaDfiClientServerExceptionTests : public ::testing::Test {
+public:
+    RsaDfiClientServerExceptionTests() {
+        setupFm(false);
+        registerExceptionTestServer();
+    }
+    ~RsaDfiClientServerExceptionTests() override {
+        unregisterExceptionTestServer();
+        teardownFm();
+    }
+};
 
 TEST_F(RsaDfiClientServerTests, TestRemoteCalculator) {
     test(testCalculator);
@@ -228,3 +391,15 @@ TEST_F(RsaDfiClientServerTests, CreateDestroyComponentWithRemoteService) {
 TEST_F(RsaDfiClientServerTests, AddRemoteServiceInRemoteService) {
     test(testAddRemoteServiceInRemoteService);
 }
+
+TEST_F(RsaDfiClientServerInterceptorTests,TestInterceptorPreExportCallReturnFalse) {
+    test(testInterceptorPreExportCallReturnFalse);
+}
+
+TEST_F(RsaDfiClientServerInterceptorTests,TestInterceptorPreProxyCallReturnFalse) {
+    test(testInterceptorPreProxyCallReturnFalse);
+}
+
+TEST_F(RsaDfiClientServerExceptionTests,TestExceptionService) {
+    testExceptionService();
+}

bundles/remote_services/remote_service_admin_dfi/gtest/src/tst_activator.c

@@ -369,4 +369,4 @@ static celix_status_t bndStop(struct activator *act, celix_bundle_context_t* ctx
     return CELIX_SUCCESS;
 }
 
-CELIX_GEN_BUNDLE_ACTIVATOR(struct activator, bndStart, bndStop);
+CELIX_GEN_BUNDLE_ACTIVATOR(struct activator, bndStart, bndStop);

bundles/remote_services/remote_service_admin_dfi/src/export_registration_dfi.c

@@ -154,7 +154,7 @@ void exportRegistration_waitTillNotUsed(export_registration_t *export) {
     celixThreadMutex_unlock(&export->mutex);
 }
 
-celix_status_t exportRegistration_call(export_registration_t *export, char *data, int datalength, celix_properties_t *metadata, char **responseOut, int *responseLength) {
+celix_status_t exportRegistration_call(export_registration_t *export, char *data, int datalength, celix_properties_t **metadata, char **responseOut, int *responseLength) {
     int status = CELIX_SUCCESS;
 
     char* response = NULL;
@@ -164,11 +164,12 @@ celix_status_t exportRegistration_call(export_registration_t *export, char *data
     const char *sig;
     if (js_request) {
         if (json_unpack(js_request, "{s:s}", "m", &sig) == 0) {
-            bool cont = remoteInterceptorHandler_invokePreExportCall(export->interceptorsHandler, export->exportReference.endpoint->properties, sig, &metadata);
+            bool cont = remoteInterceptorHandler_invokePreExportCall(export->interceptorsHandler, export->exportReference.endpoint->properties, sig, metadata);
             if (cont) {
                 celixThreadMutex_lock(&export->mutex);
                 if (export->active && export->service != NULL) {
-                    status = jsonRpc_call(export->intf, export->service, data, &response);
+                    int rc = jsonRpc_call(export->intf, export->service, data, &response);
+                    status = (rc != 0) ? CELIX_BUNDLE_EXCEPTION : CELIX_SUCCESS;
                 } else if (!export->active) {
                     status = CELIX_ILLEGAL_STATE;
                     celix_logHelper_warning(export->helper, "Cannot call an inactive service export");
@@ -178,7 +179,7 @@ celix_status_t exportRegistration_call(export_registration_t *export, char *data
                 }
                 celixThreadMutex_unlock(&export->mutex);
 
-                remoteInterceptorHandler_invokePostExportCall(export->interceptorsHandler, export->exportReference.endpoint->properties, sig, metadata);
+                remoteInterceptorHandler_invokePostExportCall(export->interceptorsHandler, export->exportReference.endpoint->properties, sig, *metadata);
             }
             *responseOut = response;
 

bundles/remote_services/remote_service_admin_dfi/src/export_registration_dfi.h

@@ -31,7 +31,7 @@ void exportRegistration_destroy(export_registration_t *registration);
 celix_status_t exportRegistration_start(export_registration_t *registration);
 void exportRegistration_setActive(export_registration_t *registration, bool active);
 
-celix_status_t exportRegistration_call(export_registration_t *export, char *data, int datalength, celix_properties_t *metadata, char **response, int *responseLength);
+celix_status_t exportRegistration_call(export_registration_t *export, char *data, int datalength, celix_properties_t **metadata, char **response, int *responseLength);
 
 void exportRegistration_increaseUsage(export_registration_t *export);
 void exportRegistration_decreaseUsage(export_registration_t *export);

bundles/remote_services/remote_service_admin_dfi/src/import_registration_dfi.c

@@ -315,14 +315,17 @@ static void importRegistration_proxyFunc(void *userData, void *args[], void *ret
     struct method_entry *entry = userData;
     import_registration_t *import = *((void **)args[0]);
 
+    *(int *) returnVal = CELIX_SUCCESS;
+
     if (import == NULL || import->send == NULL) {
         status = CELIX_ILLEGAL_ARGUMENT;
     }
 
 
     char *invokeRequest = NULL;
     if (status == CELIX_SUCCESS) {
-        status = jsonRpc_prepareInvokeRequest(entry->dynFunc, entry->id, args, &invokeRequest);
+        int rc = jsonRpc_prepareInvokeRequest(entry->dynFunc, entry->id, args, &invokeRequest);
+        status = (rc != 0) ? CELIX_BUNDLE_EXCEPTION : CELIX_SUCCESS;
         //printf("Need to send following json '%s'\n", invokeRequest);
     }
 
@@ -334,17 +337,31 @@ static void importRegistration_proxyFunc(void *userData, void *args[], void *ret
         celix_properties_t *metadata = NULL;
         bool cont = remoteInterceptorHandler_invokePreProxyCall(import->interceptorsHandler, import->endpoint->properties, entry->name, &metadata);
         if (cont) {
-            import->send(import->sendHandle, import->endpoint, invokeRequest, metadata, &reply, &rc);
+            status = import->send(import->sendHandle, import->endpoint, invokeRequest, metadata, &reply, &rc);
             //printf("request sended. got reply '%s' with status %i\n", reply, rc);
 
-            if (rc == 0 && dynFunction_hasReturn(entry->dynFunc)) {
+            if (status == CELIX_SUCCESS && rc == CELIX_SUCCESS && dynFunction_hasReturn(entry->dynFunc)) {
                 //fjprintf("Handling reply '%s'\n", reply);
-                status = jsonRpc_handleReply(entry->dynFunc, reply, args);
+                int rsErrno = CELIX_SUCCESS;
+                int retVal = jsonRpc_handleReply(entry->dynFunc, reply, args, &rsErrno);
+                if(retVal != 0) {
+                    status = CELIX_BUNDLE_EXCEPTION;
+                } else if (rsErrno != CELIX_SUCCESS) {
+                    //return the invocation error of remote service function
+                    *(int *) returnVal = rsErrno;
+                }
+            } else if (rc != CELIX_SUCCESS) {
+                *(int *) returnVal = rc;
             }
 
-            *(int *) returnVal = rc;
-
             remoteInterceptorHandler_invokePostProxyCall(import->interceptorsHandler, import->endpoint->properties, entry->name, metadata);
+        } else {
+            *(int *) returnVal = CELIX_INTERCEPTOR_EXCEPTION;
+        }
+
+        //free metadata
+        if(metadata != NULL) {
+            celix_properties_destroy(metadata);
         }
 
         if (import->logFile != NULL) {
@@ -362,6 +379,7 @@ static void importRegistration_proxyFunc(void *userData, void *args[], void *ret
 
     if (status != CELIX_SUCCESS) {
         //TODO log error
+        *(int *) returnVal = status;
     }
 }
 

bundles/remote_services/remote_service_admin_dfi/src/import_registration_dfi.h

@@ -25,7 +25,7 @@
 
 #include <celix_errno.h>
 
-typedef void (*send_func_type)(void *handle, endpoint_description_t *endpointDescription, char *request, celix_properties_t *metadata, char **reply, int* replyStatus);
+typedef celix_status_t (*send_func_type)(void *handle, endpoint_description_t *endpointDescription, char *request, celix_properties_t *metadata, char **reply, int* replyStatus);
 
 celix_status_t importRegistration_create(
         celix_bundle_context_t *context,

bundles/remote_services/remote_service_admin_dfi/src/remote_service_admin_dfi.c

@@ -501,7 +501,7 @@ static int remoteServiceAdmin_callback(struct mg_connection *conn) {
 
             char *response = NULL;
             int responceLength = 0;
-            int rc = exportRegistration_call(export, data, -1, metadata, &response, &responceLength);
+            int rc = exportRegistration_call(export, data, -1, &metadata, &response, &responceLength);
             if (rc != CELIX_SUCCESS) {
                 RSA_LOG_ERROR(rsa, "Error trying to invoke remove service, got error %i\n", rc);
             }
@@ -534,11 +534,14 @@ static int remoteServiceAdmin_callback(struct mg_connection *conn) {
 
             free(data);
             exportRegistration_decreaseUsage(export);
-
-            //TODO free metadata?
         }
     }
 
+    //free metadata
+    if(metadata != NULL) {
+        celix_properties_destroy(metadata);
+    }
+
     return result;
 }
 
@@ -957,7 +960,7 @@ static celix_status_t remoteServiceAdmin_send(void *handle, endpoint_description
         fputc('\0', get.stream);
         fclose(get.stream);
         *reply = get.buf;
-        *replyStatus = res;
+        *replyStatus = (res == CURLE_OK) ? CELIX_SUCCESS:CELIX_ERROR_MAKE(CELIX_FACILITY_HTTP,res);
 
         curl_easy_cleanup(curl);
         curl_slist_free_all(metadataHeader);