Merge pull request #699 from apache/feature/dfi-cleanup

Feature/dfi cleanup

CMakeLists.txt

@@ -221,7 +221,10 @@ install(EXPORT celix NAMESPACE Celix:: DESTINATION share/celix/cmake FILE Target
 install_celix_targets(celix NAMESPACE Celix:: DESTINATION share/celix/cmake FILE CelixTargets COMPONENT cmake)
 
 #install celix cmake modules
-install(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/cmake/Modules/ DESTINATION share/celix/cmake/Modules)
+option(INSTALL_FIND_MODULES "Whether to install Find modules defined by Celix" ON)
+if (INSTALL_FIND_MODULES)
+    install(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/cmake/Modules/ DESTINATION share/celix/cmake/Modules)
+endif ()
 install(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/cmake/cmake_celix/ DESTINATION share/celix/cmake/cmake_celix)
 
 file(GENERATE

bundles/remote_services/remote_service_admin_dfi/src/export_registration_dfi.c

@@ -29,6 +29,8 @@
 #include "dfi_utils.h"
 #include "remote_interceptors_handler.h"
 
+#include <string.h>
+
 struct export_reference {
     endpoint_description_t *endpoint; //owner
     service_reference_pt reference;
@@ -109,8 +111,7 @@ celix_status_t exportRegistration_create(celix_log_helper_t *helper, service_ref
 
     if (status == CELIX_SUCCESS) {
         /* Add the interface version as a property in the properties_map */
-        char* intfVersion = NULL;
-        dynInterface_getVersionString(reg->intf, &intfVersion);
+        const char* intfVersion = dynInterface_getVersionString(reg->intf);
         const char *serviceVersion = celix_properties_get(endpoint->properties,(char*) CELIX_FRAMEWORK_SERVICE_VERSION, NULL);
         if (serviceVersion != NULL) {
             if(strcmp(serviceVersion,intfVersion)!=0){
@@ -190,8 +191,7 @@ celix_status_t exportRegistration_call(export_registration_t *export, char *data
             //printf("calling for '%s'\n");
             if (export->logFile != NULL) {
                 static int callCount = 0;
-                char *name = NULL;
-                dynInterface_getName(export->intf, &name);
+                const char *name = dynInterface_getName(export->intf);
                 fprintf(export->logFile, "REMOTE CALL %i\n\tservice=%s\n\tservice_id=%s\n\trequest_payload=%s\n\trequest_response=%s\n\tstatus=%i\n", callCount, name, export->servId, data, response, status);
                 fflush(export->logFile);
                 callCount += 1;

bundles/remote_services/remote_service_admin_dfi/src/import_registration_dfi.c

@@ -20,7 +20,8 @@
 #include <stdlib.h>
 #include <json_rpc.h>
 #include <assert.h>
-#include "version.h"
+#include "celix_version.h"
+#include "celix_stdlib_cleanup.h"
 #include "dyn_interface.h"
 #include "import_registration.h"
 #include "import_registration_dfi.h"
@@ -32,7 +33,7 @@ struct import_registration {
     celix_bundle_context_t *context;
     endpoint_description_t * endpoint; //TODO owner? -> free when destroyed
     const char *classObject; //NOTE owned by endpoint
-    version_pt version;
+    celix_version_t* version;
 
     send_func_type send;
     void *sendHandle;
@@ -87,8 +88,11 @@ celix_status_t importRegistration_create(
     remoteInterceptorsHandler_create(context, &reg->interceptorsHandler);
 
     celixThreadMutex_create(&reg->proxiesMutex, NULL);
-    status = version_createVersionFromString((char*)serviceVersion,&(reg->version));
-
+    // serviceVersion == NULL is allowed, check TEST_F(RsaDfiTests, ImportService)
+    reg->version = celix_version_createVersionFromString(serviceVersion);
+    if (serviceVersion != NULL && reg->version == NULL) {
+        status = CELIX_ILLEGAL_ARGUMENT;
+    }
     reg->factorySvcId = -1;
     reg->factory.handle = reg;
     reg->factory.getService = importRegistration_getService;
@@ -135,7 +139,7 @@ static void importRegistration_destroyCallback(void* data) {
     pthread_mutex_destroy(&import->proxiesMutex);
 
     if (import->version != NULL) {
-        version_destroy(import->version);
+        celix_version_destroy(import->version);
     }
     free(import);
 }
@@ -230,21 +234,19 @@ static celix_status_t importRegistration_createProxy(import_registration_t *impo
     }
 
     /* Check if the imported service version is compatible with the one in the consumer descriptor */
-    version_pt consumerVersion = NULL;
+    const celix_version_t* consumerVersion = dynInterface_getVersion(intf);
     bool isCompatible = false;
-    dynInterface_getVersion(intf,&consumerVersion);
-    version_isCompatible(consumerVersion,import->version,&isCompatible);
+    isCompatible = celix_version_isCompatible(consumerVersion,import->version);
 
     if(!isCompatible){
-    	char* cVerString = NULL;
-    	char* pVerString = NULL;
-    	version_toString(consumerVersion,&cVerString);
-    	version_toString(import->version,&pVerString);
-    	printf("Service version mismatch: consumer has %s, provider has %s. NOT creating proxy.\n",cVerString,pVerString);
-    	dynInterface_destroy(intf);
-    	free(cVerString);
-    	free(pVerString);
-    	status = CELIX_SERVICE_EXCEPTION;
+        celix_autofree char* cVerString = NULL;
+        celix_autofree char* pVerString = NULL;
+        cVerString = celix_version_toString(consumerVersion);
+        pVerString = import->version != NULL ? celix_version_toString(import->version) : NULL;
+        printf("Service version mismatch: consumer has %s, provider has %s. NOT creating proxy.\n",
+               cVerString,pVerString != NULL ? pVerString : "NA");
+        dynInterface_destroy(intf);
+        status = CELIX_SERVICE_EXCEPTION;
     }
 
     struct service_proxy *proxy = NULL;
@@ -268,8 +270,7 @@ static celix_status_t importRegistration_createProxy(import_registration_t *impo
         void **serv = proxy->service;
         serv[0] = import;
 
-        struct methods_head *list = NULL;
-        dynInterface_methods(proxy->intf, &list);
+        const struct methods_head* list = dynInterface_methods(proxy->intf);
         struct method_entry *entry = NULL;
         void (*fn)(void) = NULL;
         int index = 0;
@@ -324,7 +325,8 @@ static void importRegistration_proxyFunc(void *userData, void *args[], void *ret
         int rc = 0;
         //printf("sending request\n");
         celix_properties_t *metadata = NULL;
-        bool cont = remoteInterceptorHandler_invokePreProxyCall(import->interceptorsHandler, import->endpoint->properties, entry->name, &metadata);
+        bool cont = remoteInterceptorHandler_invokePreProxyCall(import->interceptorsHandler, import->endpoint->properties,
+                                                                dynFunction_getName(entry->dynFunc), &metadata);
         if (cont) {
             status = import->send(import->sendHandle, import->endpoint, invokeRequest, metadata, &reply, &rc);
             //printf("request sended. got reply '%s' with status %i\n", reply, rc);
@@ -343,7 +345,8 @@ static void importRegistration_proxyFunc(void *userData, void *args[], void *ret
                 *(int *) returnVal = rc;
             }
 
-            remoteInterceptorHandler_invokePostProxyCall(import->interceptorsHandler, import->endpoint->properties, entry->name, metadata);
+            remoteInterceptorHandler_invokePostProxyCall(import->interceptorsHandler, import->endpoint->properties,
+                                                         dynFunction_getName(entry->dynFunc), metadata);
         } else {
             *(int *) returnVal = CELIX_INTERCEPTOR_EXCEPTION;
         }

bundles/remote_services/rsa_rpc_json/gtest/src/RsaJsonRpcUnitTestSuite.cc

@@ -790,39 +790,6 @@ TEST_F(RsaJsonRpcEndPointUnitTestSuite, FailedToFindInterfaceDescriptor) {
     unsetenv("CELIX_FRAMEWORK_EXTENDER_PATH");
 }
 
-TEST_F(RsaJsonRpcEndPointUnitTestSuite, FailedToGetServiceVersionFromInterfaceDescriptor) {
-    celix_ei_expect_dynInterface_getVersionString(CELIX_EI_UNKNOWN_CALLER, 0, 1);
-
-    auto endpoint = CreateEndpointDescription(rpcTestSvcId);
-    long svcId = -1L;
-    auto status = rsaJsonRpc_createEndpoint(jsonRpc.get(), endpoint, &svcId);
-    EXPECT_EQ(CELIX_SUCCESS, status);
-
-    celix_bundleContext_waitForEvents(ctx.get());//wait for async endpoint creation
-
-    unsigned int serialProtoId = GenerateSerialProtoId();
-    celix_properties_t *metadata = celix_properties_create();
-    celix_properties_setLong(metadata, "SerialProtocolId", serialProtoId);
-
-    auto found = celix_bundleContext_useService(ctx.get(), RSA_REQUEST_HANDLER_SERVICE_NAME, metadata, [](void *handle, void *svc) {
-        celix_properties_t *metadata = static_cast< celix_properties_t *>(handle);//unused
-        auto reqHandler = static_cast<rsa_request_handler_service_t*>(svc);
-        EXPECT_NE(nullptr, reqHandler);
-        struct iovec request{};
-        request.iov_base =  (char *)"{\n    \"m\": \"test\",\n    \"a\": []\n}";
-        request.iov_len = strlen((char*)request.iov_base);
-        struct iovec reply{nullptr,0};
-        EXPECT_EQ(CELIX_ILLEGAL_STATE, reqHandler->handleRequest(reqHandler->handle, metadata, &request, &reply));
-        free(reply.iov_base);
-    });
-    EXPECT_TRUE(found);
-
-    celix_properties_destroy(metadata);
-
-    rsaJsonRpc_destroyEndpoint(jsonRpc.get(), svcId);
-    endpointDescription_destroy(endpoint);
-}
-
 TEST_F(RsaJsonRpcEndPointUnitTestSuite, ServiceVersionMismatched) {
     auto endpoint = CreateEndpointDescription(rpcTestSvcId);
     celix_properties_set(endpoint->properties, CELIX_FRAMEWORK_SERVICE_VERSION, "2.0.0");//Its 1.0.0 in the interface descriptor

bundles/remote_services/rsa_rpc_json/src/rsa_json_rpc_endpoint_impl.c

@@ -29,6 +29,7 @@
 #include <sys/uio.h>
 #include <jansson.h>
 #include <assert.h>
+#include <string.h>
 
 struct rsa_json_rpc_endpoint {
     celix_bundle_context_t* ctx;
@@ -172,13 +173,7 @@ static void rsaJsonRpcEndpoint_addSvcWithOwner(void *handle, void *service,
     }
 
     // Check version
-    char *intfVersion = NULL;
-    int ret = dynInterface_getVersionString(intfType, &intfVersion);
-    if (ret != 0) {
-        celix_logHelper_logTssErrors(endpoint->logHelper, CELIX_LOG_LEVEL_ERROR);
-        celix_logHelper_error(endpoint->logHelper, "Endpoint: Error getting interface version from the descriptor for %s.", serviceName);
-        return;
-    }
+    const char* intfVersion = dynInterface_getVersionString(intfType);
     const char *serviceVersion = celix_properties_get(endpoint->endpointDesc->properties,CELIX_FRAMEWORK_SERVICE_VERSION, NULL);
     if (serviceVersion == NULL) {
         celix_logHelper_error(endpoint->logHelper, "Endpoint: Error getting service version for %s.", serviceName);

bundles/remote_services/rsa_rpc_json/src/rsa_json_rpc_impl.c

@@ -36,6 +36,7 @@
 #include <assert.h>
 #include <stdbool.h>
 #include <stddef.h>
+#include <string.h>
 
 struct rsa_json_rpc {
     celix_bundle_context_t *ctx;

bundles/remote_services/rsa_rpc_json/src/rsa_json_rpc_proxy_impl.c

@@ -29,8 +29,9 @@
 #include "celix_build_assert.h"
 #include "celix_long_hash_map.h"
 #include <sys/queue.h>
-#include <stdbool.h>
 #include <assert.h>
+#include <stdbool.h>
+#include <string.h>
 
 struct rsa_json_rpc_proxy_factory {
     celix_bundle_context_t* ctx;
@@ -212,22 +213,24 @@ static void rsaJsonRpcProxy_serviceFunc(void *userData, void *args[], void *retu
     int rc = jsonRpc_prepareInvokeRequest(entry->dynFunc, entry->id, args, &invokeRequest);
     if (rc != 0) {
         celix_logHelper_logTssErrors(proxyFactory->logHelper, CELIX_LOG_LEVEL_ERROR);
-        celix_logHelper_error(proxyFactory->logHelper, "Error preparing invoke request for %s", entry->name);
+        celix_logHelper_error(proxyFactory->logHelper, "Error preparing invoke request for %s",
+                              dynFunction_getName(entry->dynFunc));
         *(celix_status_t *)returnVal = CELIX_SERVICE_EXCEPTION;
         return;
     }
 
     struct iovec replyIovec = {NULL,0};
     celix_properties_t *metadata = celix_properties_create();
     if (metadata == NULL) {
-        celix_logHelper_error(proxyFactory->logHelper,"Error creating metadata for %s", entry->name);
+        celix_logHelper_error(proxyFactory->logHelper,"Error creating metadata for %s",
+                              dynFunction_getName(entry->dynFunc));
         free(invokeRequest);
         *(celix_status_t *)returnVal = CELIX_ENOMEM;
         return;
     }
     celix_properties_setLong(metadata, "SerialProtocolId", proxyFactory->serialProtoId);
     bool cont = remoteInterceptorHandler_invokePreProxyCall(proxyFactory->interceptorsHandler,
-            proxyFactory->endpointDesc->properties, entry->name, &metadata);
+            proxyFactory->endpointDesc->properties, dynFunction_getName(entry->dynFunc), &metadata);
     if (cont) {
         struct iovec requestIovec = {invokeRequest,strlen(invokeRequest) + 1};
         struct rsa_request_sender_callback_data data= {
@@ -246,7 +249,8 @@ static void rsaJsonRpcProxy_serviceFunc(void *userData, void *args[], void *retu
                 if(retVal != 0) {
                     status = CELIX_SERVICE_EXCEPTION;
                     celix_logHelper_logTssErrors(proxyFactory->logHelper, CELIX_LOG_LEVEL_ERROR);
-                    celix_logHelper_error(proxyFactory->logHelper, "Error handling reply for %s", entry->name);
+                    celix_logHelper_error(proxyFactory->logHelper, "Error handling reply for %s",
+                                          dynFunction_getName(entry->dynFunc));
                 } else if (rsErrno != CELIX_SUCCESS) {
                     //return the invocation error of remote service function
                     status = rsErrno;
@@ -259,7 +263,7 @@ static void rsaJsonRpcProxy_serviceFunc(void *userData, void *args[], void *retu
             celix_logHelper_error(proxyFactory->logHelper,"Service proxy send request failed. %d", status);
         }
         remoteInterceptorHandler_invokePostProxyCall(proxyFactory->interceptorsHandler,
-                proxyFactory->endpointDesc->properties, entry->name, metadata);
+                proxyFactory->endpointDesc->properties, dynFunction_getName(entry->dynFunc), metadata);
     } else {
         celix_logHelper_error(proxyFactory->logHelper, "%s has been intercepted.", proxyFactory->endpointDesc->serviceName);
         status = CELIX_INTERCEPTOR_EXCEPTION;
@@ -317,9 +321,8 @@ static celix_status_t rsaJsonRpcProxy_create(rsa_json_rpc_proxy_factory_t *proxy
         celix_logHelper_error(proxyFactory->logHelper, "Proxy: Error converting service version type. %d.", status);
         return status;
     }
-    celix_version_t *consumerVersion = NULL;
+    const celix_version_t *consumerVersion = dynInterface_getVersion(intfType);
     bool isCompatible = false;
-    dynInterface_getVersion(intfType,&consumerVersion);
     isCompatible = celix_version_isCompatible(consumerVersion, providerVersion);
     if(!isCompatible){
         celix_logHelper_error(proxyFactory->logHelper, "Proxy: Service version mismatch, consumer has %d.%d.%d, provider has %s.",
@@ -336,15 +339,15 @@ static celix_status_t rsaJsonRpcProxy_create(rsa_json_rpc_proxy_factory_t *proxy
     }
     celix_autofree void **service = (void **)proxy->service;
     service[0] = proxy;
-    struct methods_head *list = NULL;
-    dynInterface_methods(intfType, &list);
+    const struct methods_head* list = dynInterface_methods(intfType);
     struct method_entry *entry = NULL;
     void (*fn)(void) = NULL;
     int index = 0;
     TAILQ_FOREACH(entry, list, entries) {
         int rc = dynFunction_createClosure(entry->dynFunc, rsaJsonRpcProxy_serviceFunc, entry, &fn);
         if (rc != 0) {
-            celix_logHelper_error(proxyFactory->logHelper, "Proxy: Failed to create closure for service function %s.", entry->name);
+            celix_logHelper_error(proxyFactory->logHelper, "Proxy: Failed to create closure for service function %s.",
+                                  dynFunction_getName(entry->dynFunc));
             return CELIX_SERVICE_EXCEPTION;
         }
         service[++index] = fn;

cmake/celix_project/CodeCoverage.cmake

@@ -70,8 +70,8 @@ IF(ENABLE_CODE_COVERAGE)
 
     # Setup compiler options
     ADD_DEFINITIONS(--coverage)
-    set(CMAKE_SHARED_LINKER_FLAGS "--coverage")
-    set(CMAKE_EXE_LINKER_FLAGS "--coverage")
+    set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} --coverage")
+    set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} --coverage")
 
     IF(NOT TARGET coverage)
         add_custom_target(coverage

conanfile.py

@@ -47,6 +47,7 @@ class CelixConan(ConanFile):
         "enable_address_sanitizer": False,
         "enable_undefined_sanitizer": False,
         "enable_thread_sanitizer": False,
+        "install_find_modules": False,
         "build_all": False,
         "build_http_admin": False,
         "build_log_service": False,
@@ -338,6 +339,8 @@ def generate(self):
             lst = [x.ref.name for x in self.requires.values()]
             if "mdnsresponder" in lst:
                 tc.cache_variables["BUILD_ERROR_INJECTOR_MDNSRESPONDER"] = "ON"
+            if "jansson" in lst:
+                tc.cache_variables["BUILD_ERROR_INJECTOR_JANSSON"] = "ON"
         tc.cache_variables["CELIX_ERR_BUFFER_SIZE"] = str(self.options.celix_err_buffer_size)
         # tc.cache_variables["CMAKE_PROJECT_Celix_INCLUDE"] = os.path.join(self.build_folder, "conan_paths.cmake")
         # the following is workaround for https://github.com/conan-io/conan/issues/7192

libs/dfi/CMakeLists.txt

@@ -29,12 +29,14 @@ if (CELIX_DFI)
 			src/dyn_message.c
 			src/json_serializer.c
 			src/json_rpc.c
+			src/dyn_descriptor.c
 	)
 
 	add_library(dfi SHARED ${SOURCES})
 	target_include_directories(dfi PUBLIC
 			$<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}/include>
-			$<INSTALL_INTERFACE:include/celix/dfi>)
+			$<INSTALL_INTERFACE:include/celix/dfi>
+			PRIVATE ${CMAKE_CURRENT_LIST_DIR}/src)
 	target_link_libraries(dfi PRIVATE libffi::libffi)
 	target_link_libraries(dfi PUBLIC jansson::jansson Celix::utils)##The public header file(dyn_interface.h) Celix::utils(celix_version.h)
 	set_target_properties(dfi PROPERTIES
@@ -60,6 +62,14 @@ if (CELIX_DFI)
 		add_subdirectory(error_injector)
 	endif ()
 	if (ENABLE_TESTING)
+		add_library(dfi_cut STATIC ${SOURCES})
+		target_compile_definitions(dfi_cut PUBLIC CELIX_DFI_STATIC_DEFINE)
+		target_include_directories(dfi_cut PUBLIC
+				${CMAKE_CURRENT_LIST_DIR}/src
+				${CMAKE_CURRENT_LIST_DIR}/include
+				${CMAKE_BINARY_DIR}/celix/gen/includes/dfi
+				)
+		target_link_libraries(dfi_cut PUBLIC libffi::libffi jansson::jansson Celix::utils)
 		add_subdirectory(gtest)
 	endif(ENABLE_TESTING)
 endif (CELIX_DFI)

libs/dfi/error_injector/CMakeLists.txt

@@ -15,4 +15,5 @@
 # specific language governing permissions and limitations
 # under the License.
 
-add_subdirectory(dfi)
+add_subdirectory(dfi)
+add_subdirectory(ffi)

libs/dfi/error_injector/dfi/CMakeLists.txt

@@ -23,6 +23,5 @@ target_link_libraries(dfi_ei PUBLIC Celix::error_injector Celix::dfi)
 target_link_options(dfi_ei INTERFACE
         LINKER:--wrap,dynFunction_createClosure
         LINKER:--wrap,jsonRpc_prepareInvokeRequest
-        LINKER:--wrap,dynInterface_getVersionString
 )
 add_library(Celix::dfi_ei ALIAS dfi_ei)

libs/dfi/error_injector/dfi/include/dfi_ei.h

@@ -28,8 +28,6 @@ CELIX_EI_DECLARE(dynFunction_createClosure, int);
 
 CELIX_EI_DECLARE(jsonRpc_prepareInvokeRequest, int);
 
-CELIX_EI_DECLARE(dynInterface_getVersionString, int);
-
 #ifdef __cplusplus
 }
 #endif

libs/dfi/error_injector/dfi/src/dfi_ei.cc

@@ -35,11 +35,4 @@ int __wrap_jsonRpc_prepareInvokeRequest(dyn_function_type *func, const char *id,
     return __real_jsonRpc_prepareInvokeRequest(func, id, args, out);
 }
 
-int __real_dynInterface_getVersionString(dyn_interface_type *dynInterface, char **out);
-CELIX_EI_DEFINE(dynInterface_getVersionString, int)
-int __wrap_dynInterface_getVersionString(dyn_interface_type *dynInterface, char **out) {
-    CELIX_EI_IMPL(dynInterface_getVersionString);
-    return __real_dynInterface_getVersionString(dynInterface, out);
-}
-
 }