Merge pull request #721 from apache/feature/674-improve-properties

Feature/674 improve properties

CHANGES.md

@@ -69,10 +69,12 @@ limitations under the License.
 - It is no longer possible to use the `celix_bundleContext_useService*` functions or `celix::BundleContxt::useService*` 
   methods on the Celix event thread. The calls will now immediately return and log an error if called on the
   Celix event thread.
+- Apache Celix filter now use the underlying properties value types for matching. This means that it is more important
+  to add service properties with the correct type.
 
 ## New Features
 
-- Basic type support for value in celix Properties.
+- Type support for value in celix Properties, including support for arrays.
 
 # Noteworthy Changes for 2.4.0 (2023-09-27)
 

NOTICE

@@ -1,5 +1,5 @@
 Apache Celix
-Copyright [2010-2019] The Apache Software Foundation
+Copyright [2010-2024] The Apache Software Foundation
 
 This product includes software developed at
 The Apache Software Foundation (http://www.apache.org/).

bundles/remote_services/CMakeLists.txt

@@ -20,6 +20,7 @@ if (REMOTE_SERVICE_ADMIN)
     add_subdirectory(remote_services_api)
     add_subdirectory(rsa_spi)
     add_subdirectory(rsa_common)
+    add_subdirectory(rsa_utils)
     add_subdirectory(rsa_dfi_utils)
     add_subdirectory(discovery_common)
     add_subdirectory(discovery_configured)

bundles/remote_services/remote_service_admin_dfi/CMakeLists.txt

@@ -35,6 +35,7 @@ if (RSA_REMOTE_SERVICE_ADMIN_DFI)
             src/import_registration_dfi.c
             )
     target_link_libraries(rsa_dfi PRIVATE
+            Celix::rsa_utils
             Celix::rsa_dfi_utils
             Celix::dfi
             Celix::log_helper

bundles/remote_services/remote_service_admin_dfi/src/import_registration_dfi.c

@@ -17,16 +17,20 @@
  * under the License.
  */
 
+#include "import_registration_dfi.h"
+
 #include <stdlib.h>
 #include <json_rpc.h>
 #include <assert.h>
+
 #include "celix_version.h"
 #include "celix_stdlib_cleanup.h"
+#include "celix_rsa_utils.h"
+#include "celix_constants.h"
 #include "dyn_interface.h"
 #include "import_registration.h"
-#include "import_registration_dfi.h"
-#include "remote_service_admin_dfi.h"
 #include "remote_interceptors_handler.h"
+#include "remote_service_admin_dfi.h"
 #include "remote_service_admin_dfi_constants.h"
 
 struct import_registration {
@@ -155,8 +159,13 @@ void importRegistration_destroy(import_registration_t *import) {
 }
 
 celix_status_t importRegistration_start(import_registration_t *import) {
-    celix_properties_t *props =  celix_properties_copy(import->endpoint->properties);
-    import->factorySvcId = celix_bundleContext_registerServiceFactoryAsync(import->context, &import->factory, import->classObject, props);
+    celix_properties_t* svcProperties = NULL;
+    celix_status_t status = celix_rsaUtils_createServicePropertiesFromEndpointProperties(import->endpoint->properties, &svcProperties);
+    if (status != CELIX_SUCCESS) {
+        return status;
+    }
+
+    import->factorySvcId = celix_bundleContext_registerServiceFactoryAsync(import->context, &import->factory, import->classObject, svcProperties);
     return import->factorySvcId >= 0 ? CELIX_SUCCESS : CELIX_ILLEGAL_STATE;
 }
 

bundles/remote_services/remote_service_admin_shm_v2/rsa_shm/src/rsa_shm_impl.c

@@ -288,7 +288,7 @@ celix_status_t rsaShm_exportService(rsa_shm_t *admin, char *serviceId,
         return CELIX_ILLEGAL_ARGUMENT;
     }
 
-    celix_array_list_t *references = NULL;
+    celix_autoptr(celix_array_list_t) references = NULL;
     service_reference_pt reference = NULL;
     char filter[32] = {0};// It is longer than the size of "service.id" + serviceId
     snprintf(filter, sizeof(filter), "(%s=%s)", (char *) CELIX_FRAMEWORK_SERVICE_ID, serviceId);
@@ -298,10 +298,11 @@ celix_status_t rsaShm_exportService(rsa_shm_t *admin, char *serviceId,
        return status;
     }
     //We get reference with serviceId, so the size of references must be less than or equal to 1.
-    reference = celix_arrayList_get(references, 0);
-    celix_arrayList_destroy(references);
+    if (celix_arrayList_size(references) == 1) {
+        reference = celix_arrayList_get(references, 0);
+    }
     if (reference == NULL) {
-        celix_logHelper_error(admin->logHelper, "Expect a reference for service id %s.", serviceId);
+        celix_logHelper_error(admin->logHelper, "Expect a exactly one reference for service id %s. Got %i", serviceId, celix_arrayList_size(references));
         return CELIX_ILLEGAL_STATE;
     }
     celix_auto(celix_service_ref_guard_t) ref = celix_ServiceRefGuard_init(admin->context, reference);

bundles/remote_services/rsa_rpc_json/CMakeLists.txt

@@ -36,6 +36,7 @@ if (RSA_JSON_RPC)
             Celix::log_helper
             Celix::framework
             Celix::utils
+            Celix::rsa_utils
             jansson::jansson
             )
 

bundles/remote_services/rsa_rpc_json/src/rsa_json_rpc_proxy_impl.c

@@ -18,21 +18,24 @@
  */
 
 #include "rsa_json_rpc_proxy_impl.h"
-#include "rsa_request_sender_tracker.h"
-#include "json_rpc.h"
-#include "endpoint_description.h"
-#include "celix_stdlib_cleanup.h"
-#include "celix_log_helper.h"
-#include "dfi_utils.h"
-#include "celix_version.h"
-#include "celix_constants.h"
-#include "celix_build_assert.h"
-#include "celix_long_hash_map.h"
-#include <sys/queue.h>
+
 #include <assert.h>
 #include <stdbool.h>
+#include <sys/queue.h>
 #include <string.h>
 
+#include "celix_build_assert.h"
+#include "celix_constants.h"
+#include "celix_log_helper.h"
+#include "celix_long_hash_map.h"
+#include "celix_rsa_utils.h"
+#include "celix_stdlib_cleanup.h"
+#include "celix_version.h"
+#include "dfi_utils.h"
+#include "endpoint_description.h"
+#include "json_rpc.h"
+#include "rsa_request_sender_tracker.h"
+
 struct rsa_json_rpc_proxy_factory {
     celix_bundle_context_t* ctx;
     celix_log_helper_t *logHelper;
@@ -70,10 +73,15 @@ static celix_status_t rsaJsonRpcProxy_create(rsa_json_rpc_proxy_factory_t *proxy
 static void rsaJsonRpcProxy_destroy(rsa_json_rpc_proxy_t *proxy);
 static void rsaJsonRpcProxy_unregisterFacSvcDone(void *data);
 
-celix_status_t rsaJsonRpcProxy_factoryCreate(celix_bundle_context_t* ctx, celix_log_helper_t *logHelper,
-        FILE *logFile, remote_interceptors_handler_t *interceptorsHandler,
-        const endpoint_description_t *endpointDesc, rsa_request_sender_tracker_t *reqSenderTracker,
-        long requestSenderSvcId, unsigned int serialProtoId, rsa_json_rpc_proxy_factory_t **proxyFactoryOut) {
+celix_status_t rsaJsonRpcProxy_factoryCreate(celix_bundle_context_t* ctx,
+                                             celix_log_helper_t* logHelper,
+                                             FILE* logFile,
+                                             remote_interceptors_handler_t* interceptorsHandler,
+                                             const endpoint_description_t* endpointDesc,
+                                             rsa_request_sender_tracker_t* reqSenderTracker,
+                                             long requestSenderSvcId,
+                                             unsigned int serialProtoId,
+                                             rsa_json_rpc_proxy_factory_t** proxyFactoryOut) {
     assert(ctx != NULL);
     assert(logHelper != NULL);
     assert(interceptorsHandler != NULL);
@@ -94,15 +102,15 @@ celix_status_t rsaJsonRpcProxy_factoryCreate(celix_bundle_context_t* ctx, celix_
     proxyFactory->reqSenderSvcId = requestSenderSvcId;
     proxyFactory->serialProtoId = serialProtoId;
 
-    CELIX_BUILD_ASSERT(sizeof(long) == sizeof(void*));//The hash_map uses the pointer as key, so this should be true
+    CELIX_BUILD_ASSERT(sizeof(long) == sizeof(void*)); // The hash_map uses the pointer as key, so this should be true
     celix_autoptr(celix_long_hash_map_t) proxies = proxyFactory->proxies = celix_longHashMap_create();
     if (proxyFactory->proxies == NULL) {
         celix_logHelper_error(logHelper, "Proxy: Error creating proxy map.");
         return CELIX_ENOMEM;
     }
 
-    celix_autoptr(endpoint_description_t) endpointDescCopy =
-        proxyFactory->endpointDesc = endpointDescription_clone(endpointDesc);
+    celix_autoptr(endpoint_description_t) endpointDescCopy = proxyFactory->endpointDesc =
+        endpointDescription_clone(endpointDesc);
     if (proxyFactory->endpointDesc == NULL) {
         celix_logHelper_error(logHelper, "Proxy: Failed to clone endpoint description.");
         return CELIX_ENOMEM;
@@ -111,11 +119,16 @@ celix_status_t rsaJsonRpcProxy_factoryCreate(celix_bundle_context_t* ctx, celix_
     proxyFactory->factory.handle = proxyFactory;
     proxyFactory->factory.getService = rsaJsonRpcProxy_getService;
     proxyFactory->factory.ungetService = rsaJsonRpcProxy_ungetService;
-    celix_properties_t *props =  celix_properties_copy(endpointDesc->properties);
-    assert(props != NULL);
+    celix_properties_t* svcProperties = NULL;
+    celix_status_t status =
+        celix_rsaUtils_createServicePropertiesFromEndpointProperties(endpointDesc->properties, &svcProperties);
+    if (status != CELIX_SUCCESS) {
+        return status;
+    }
+    assert(svcProperties != NULL);
     proxyFactory->factorySvcId = celix_bundleContext_registerServiceFactoryAsync(
-            ctx, &proxyFactory->factory, endpointDesc->serviceName, props);
-    if (proxyFactory->factorySvcId  < 0) {
+        ctx, &proxyFactory->factory, endpointDesc->serviceName, svcProperties);
+    if (proxyFactory->factorySvcId < 0) {
         celix_logHelper_error(logHelper, "Proxy: Error Registering proxy service.");
         return CELIX_SERVICE_EXCEPTION;
     }

bundles/remote_services/rsa_utils/CMakeLists.txt

@@ -0,0 +1,39 @@
+# 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.
+
+set(RSA_UTILS_SRC
+    src/celix_rsa_utils.c
+)
+
+add_library(rsa_utils STATIC ${RSA_UTILS_SRC})
+set_target_properties(rsa_utils PROPERTIES OUTPUT_NAME "celix_rsa_utils")
+target_include_directories(rsa_utils PRIVATE src)
+target_include_directories(rsa_utils PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}/include>)
+target_link_libraries(rsa_utils PUBLIC Celix::utils PRIVATE Celix::framework)
+
+install(TARGETS rsa_utils EXPORT celix LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} COMPONENT rsa
+        INCLUDES DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/celix/rsa_utils)
+install(DIRECTORY include/
+        DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/celix/rsa_utils/
+        COMPONENT rsa)
+
+#Setup target aliases to match external usage
+add_library(Celix::rsa_utils ALIAS rsa_utils)
+
+if (ENABLE_TESTING)
+    add_subdirectory(gtest)
+endif ()

bundles/remote_services/rsa_utils/gtest/CMakeLists.txt

@@ -0,0 +1,35 @@
+# 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.
+
+add_executable(test_rsa_utils
+        src/RsaUtilsTestSuite.cc
+)
+
+target_link_libraries(test_rsa_utils PRIVATE rsa_utils GTest::gtest GTest::gtest_main Celix::framework)
+target_include_directories(test_utils PRIVATE ../src) #for version_private (needs refactoring of test)
+add_test(NAME test_rsa_utils COMMAND test_rsa_utils)
+setup_target_for_coverage(test_rsa_utils SCAN_DIR ..)
+
+if (EI_TESTS)
+    add_executable(test_rsa_utils_ei
+            src/RsaUtilsErrorInjectionTestSuite.cc
+    )
+    target_link_libraries(test_rsa_utils_ei PRIVATE rsa_utils GTest::gtest GTest::gtest_main)
+    target_link_libraries(test_rsa_utils_ei PRIVATE properties_ei)
+    add_test(NAME test_rsa_utils_ei COMMAND test_rsa_utils_ei)
+    setup_target_for_coverage(test_rsa_utils_ei SCAN_DIR ..)
+endif ()

bundles/remote_services/rsa_utils/gtest/src/RsaUtilsErrorInjectionTestSuite.cc

@@ -0,0 +1,53 @@
+/*
+* 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 <gtest/gtest.h>
+
+#include "celix_rsa_utils.h"
+#include "celix_properties_ei.h"
+#include "celix_err.h"
+
+class RsaUtilsErrorInjectionTestSuite : public ::testing::Test {
+ public:
+   RsaUtilsErrorInjectionTestSuite() {
+       celix_ei_expect_celix_properties_copy(nullptr, 0, nullptr);
+       celix_err_printErrors(stderr, nullptr, nullptr);
+   }
+   ~RsaUtilsErrorInjectionTestSuite() override = default;
+};
+
+TEST_F(RsaUtilsErrorInjectionTestSuite, PropertiesCopyFailureTest) {
+    // Given an error injection for celix_properties_copy
+    celix_ei_expect_celix_properties_copy((void*)celix_rsaUtils_createServicePropertiesFromEndpointProperties, 0, nullptr);
+
+    // And an endpointProperties
+    celix_autoptr(celix_properties_t) endpointProperties = celix_properties_create();
+    EXPECT_NE(endpointProperties, nullptr);
+
+    // When calling celix_rsaUtils_createServicePropertiesFromEndpointProperties
+    celix_properties_t* serviceProperties = nullptr;
+    celix_status_t status =
+        celix_rsaUtils_createServicePropertiesFromEndpointProperties(endpointProperties, &serviceProperties);
+
+    // Then the status is CELIX_ENOMEM
+    EXPECT_EQ(status, CELIX_ENOMEM);
+
+    // And the serviceProperties is nullptr
+    EXPECT_EQ(serviceProperties, nullptr);
+}