From 3437c04516183cf98bd6c1fed8df80d77b3c0f12 Mon Sep 17 00:00:00 2001 From: PengZheng Date: Tue, 2 Apr 2024 21:29:29 +0800 Subject: [PATCH 1/3] gh-674: Add missing array list support to several celix_properties functions, add more tests. --- .../src/PropertiesErrorInjectionTestSuite.cc | 45 +++++++++++++- libs/utils/gtest/src/PropertiesTestSuite.cc | 58 +++++++++++++++++-- libs/utils/include/celix_properties.h | 13 ++--- libs/utils/src/properties.c | 43 ++++++-------- 4 files changed, 119 insertions(+), 40 deletions(-) diff --git a/libs/utils/gtest/src/PropertiesErrorInjectionTestSuite.cc b/libs/utils/gtest/src/PropertiesErrorInjectionTestSuite.cc index da2e2af72..162a86fcd 100644 --- a/libs/utils/gtest/src/PropertiesErrorInjectionTestSuite.cc +++ b/libs/utils/gtest/src/PropertiesErrorInjectionTestSuite.cc @@ -19,15 +19,18 @@ */ #include +#include #include "celix/Properties.h" #include "celix_cleanup.h" +#include "celix_convert_utils.h" #include "celix_err.h" #include "celix_properties.h" #include "celix_properties_private.h" #include "celix_utils_private_constants.h" #include "celix_version.h" +#include "asprintf_ei.h" #include "celix_array_list_ei.h" #include "celix_string_hash_map_ei.h" #include "celix_utils_ei.h" @@ -40,6 +43,8 @@ class PropertiesErrorInjectionTestSuite : public ::testing::Test { PropertiesErrorInjectionTestSuite() = default; ~PropertiesErrorInjectionTestSuite() override { celix_err_resetErrors(); + celix_ei_expect_open_memstream(nullptr, 0, nullptr); + celix_ei_expect_asprintf(nullptr, 0, -1); celix_ei_expect_malloc(nullptr, 0, nullptr); celix_ei_expect_celix_stringHashMap_createWithOptions(nullptr, 0, nullptr); celix_ei_expect_celix_arrayList_copy(nullptr, 0, nullptr); @@ -140,6 +145,20 @@ TEST_F(PropertiesErrorInjectionTestSuite, SetFailureTest) { // Then the celix_properties_set call fails ASSERT_EQ(celix_properties_set(props, "key", "value"), CELIX_ENOMEM); + celix_ei_expect_celix_utils_strdup((void*)celix_properties_createString, 0, nullptr); + ASSERT_EQ(CELIX_ENOMEM, celix_properties_setLong(props, "key", 1000)); + + celix_ei_expect_celix_utils_strdup((void*)celix_properties_createString, 0, nullptr); + ASSERT_EQ(CELIX_ENOMEM, celix_properties_setDouble(props, "key", 1.2)); + + double largeNumber = 123456789012345.0; + celix_ei_expect_asprintf((void*)celix_properties_setDouble, 3, -1); + ASSERT_EQ(CELIX_ENOMEM, celix_properties_setDouble(props, "key", largeNumber)); + + celix_autoptr(celix_array_list_t) list = celix_arrayList_createLongArray(); + celix_ei_expect_open_memstream((void*)celix_utils_arrayListToString, 1, nullptr); + ASSERT_EQ(CELIX_ENOMEM, celix_properties_setArrayList(props, "key", list)); + // C++ API // Given a celix properties object with a filled optimization cache celix::Properties cxxProps{}; @@ -309,6 +328,7 @@ TEST_F(PropertiesErrorInjectionTestSuite, GetAsArrayWithArrayListCopyFailedTest) celix_properties_setArrayList(props, "doubleArray", doubleList); celix_properties_setArrayList(props, "boolArray", boolList); celix_properties_setArrayList(props, "versionArray", versionList); + celix_properties_setString(props, "string", "Hello world"); // When a celix_arrayList_createWithOptions error injection is set for celix_properties_getAsStringArrayList celix_ei_expect_celix_arrayList_copy((void*)celix_properties_getAsStringArrayList, 1, nullptr); @@ -354,6 +374,13 @@ TEST_F(PropertiesErrorInjectionTestSuite, GetAsArrayWithArrayListCopyFailedTest) status = celix_properties_getAsVersionArrayList(props, "versionArray", nullptr, &versions); ASSERT_EQ(status, CELIX_ENOMEM); ASSERT_EQ(nullptr, versions); + + + celix_ei_expect_open_memstream((void*)celix_utils_convertStringToVersionArrayList, 1, nullptr); + versions = nullptr; + status = celix_properties_getAsVersionArrayList(props, "string", nullptr, &versions); + ASSERT_EQ(status, CELIX_ENOMEM); + ASSERT_EQ(nullptr, versions); } TEST_F(PropertiesErrorInjectionTestSuite, SetArrayWithArrayListCopyFailedTest) { @@ -411,7 +438,7 @@ TEST_F(PropertiesErrorInjectionTestSuite, LoadFromStringFailureTest) { celix_err_resetErrors(); } -TEST_F(PropertiesErrorInjectionTestSuite, LoadSetVersionFailureTest) { +TEST_F(PropertiesErrorInjectionTestSuite, SetVersionFailureTest) { // Given a celix properties object celix_autoptr(celix_properties_t) props = celix_properties_create(); // And a version object @@ -424,4 +451,20 @@ TEST_F(PropertiesErrorInjectionTestSuite, LoadSetVersionFailureTest) { //And a celix err msg is pushed ASSERT_EQ(1, celix_err_getErrorCount()); celix_err_resetErrors(); + + celix_autoptr(celix_version_t) version2 = celix_version_create(1, 2, 3, "aaaaaaaaaaaaaaaaaaaaaaaaaa"); + celix_ei_expect_asprintf((void*) celix_version_toString, 0, -1); + status = celix_properties_setVersion(props, "key", version2); + ASSERT_EQ(status, CELIX_ENOMEM); + ASSERT_STREQ("Cannot fill property entry", celix_err_popLastError()); + ASSERT_STREQ("Failed to allocate memory for celix_version_toString", celix_err_popLastError()); + celix_err_resetErrors(); + + fillOptimizationCache(props); + celix_ei_expect_celix_utils_strdup((void*)celix_properties_createString, 0, nullptr); + status = celix_properties_setVersion(props, "key1", version); + ASSERT_EQ(status, CELIX_ENOMEM); + //And a celix err msg is pushed + ASSERT_EQ(1, celix_err_getErrorCount()); + celix_err_resetErrors(); } diff --git a/libs/utils/gtest/src/PropertiesTestSuite.cc b/libs/utils/gtest/src/PropertiesTestSuite.cc index 9c1825b51..7e15b0557 100644 --- a/libs/utils/gtest/src/PropertiesTestSuite.cc +++ b/libs/utils/gtest/src/PropertiesTestSuite.cc @@ -150,19 +150,23 @@ TEST_F(PropertiesTestSuite, GetSetTest) { char keyA[] = "x"; char keyB[] = "y"; char keyC[] = "z"; - char *keyD = strndup("a", 1); + char* keyD = strndup("a", 1); + const char* keyE = "e"; char valueA[] = "1"; char valueB[] = "2"; char valueC[] = "3"; char *valueD = strndup("4", 1); + char *valueE = strdup("5"); celix_properties_set(properties, keyA, valueA); celix_properties_set(properties, keyB, valueB); celix_properties_assign(properties, keyD, valueD); + celix_properties_assignString(properties, keyE, valueE); EXPECT_STREQ(valueA, celix_properties_get(properties, keyA, nullptr)); EXPECT_STREQ(valueB, celix_properties_get(properties, keyB, nullptr)); EXPECT_STREQ(valueC, celix_properties_get(properties, keyC, valueC)); EXPECT_STREQ(valueD, celix_properties_get(properties, keyD, nullptr)); + EXPECT_STREQ(valueE, celix_properties_get(properties, keyE, nullptr)); celix_properties_destroy(properties); } @@ -368,22 +372,31 @@ TEST_F(PropertiesTestSuite, GetTypeAndCopyTest) { celix_properties_setBool(props, "bool", true); auto* version = celix_version_create(1, 2, 3, nullptr); celix_properties_setVersion(props, "version", version); + celix_autoptr(celix_array_list_t) longList = celix_arrayList_createLongArray(); + celix_arrayList_addLong(longList, 1); + celix_arrayList_addLong(longList, 2); + celix_arrayList_addLong(longList, 3); + celix_properties_setArrayList(props, "array", longList); - EXPECT_EQ(5, celix_properties_size(props)); + EXPECT_EQ(6, celix_properties_size(props)); EXPECT_EQ(CELIX_PROPERTIES_VALUE_TYPE_STRING, celix_properties_getType(props, "string")); EXPECT_EQ(CELIX_PROPERTIES_VALUE_TYPE_LONG, celix_properties_getType(props, "long")); EXPECT_EQ(CELIX_PROPERTIES_VALUE_TYPE_DOUBLE, celix_properties_getType(props, "double")); EXPECT_EQ(CELIX_PROPERTIES_VALUE_TYPE_BOOL, celix_properties_getType(props, "bool")); EXPECT_EQ(CELIX_PROPERTIES_VALUE_TYPE_VERSION, celix_properties_getType(props, "version")); EXPECT_EQ(CELIX_PROPERTIES_VALUE_TYPE_UNSET, celix_properties_getType(props, "missing")); + EXPECT_EQ(CELIX_PROPERTIES_VALUE_TYPE_ARRAY_LIST, celix_properties_getType(props, "array")); + EXPECT_TRUE(celix_arrayList_equals(longList, celix_properties_getArrayList(props, "array"))); auto* copy = celix_properties_copy(props); - EXPECT_EQ(5, celix_properties_size(copy)); + EXPECT_EQ(6, celix_properties_size(copy)); EXPECT_EQ(CELIX_PROPERTIES_VALUE_TYPE_STRING, celix_properties_getType(copy, "string")); EXPECT_EQ(CELIX_PROPERTIES_VALUE_TYPE_LONG, celix_properties_getType(copy, "long")); EXPECT_EQ(CELIX_PROPERTIES_VALUE_TYPE_DOUBLE, celix_properties_getType(copy, "double")); EXPECT_EQ(CELIX_PROPERTIES_VALUE_TYPE_BOOL, celix_properties_getType(copy, "bool")); EXPECT_EQ(CELIX_PROPERTIES_VALUE_TYPE_VERSION, celix_properties_getType(copy, "version")); + EXPECT_EQ(CELIX_PROPERTIES_VALUE_TYPE_ARRAY_LIST, celix_properties_getType(copy, "array")); + EXPECT_TRUE(celix_arrayList_equals(longList, celix_properties_getArrayList(copy, "array"))); celix_version_destroy(version); celix_properties_destroy(props); @@ -572,6 +585,8 @@ TEST_F(PropertiesTestSuite, SetWithCopyTest) { celix_properties_assign(props, celix_utils_strdup("key"), celix_utils_strdup("value2")); EXPECT_EQ(1, celix_properties_size(props)); celix_properties_destroy(props); + + EXPECT_EQ(CELIX_SUCCESS, celix_properties_assign(nullptr, celix_utils_strdup("key"), celix_utils_strdup("value2"))); } TEST_F(PropertiesTestSuite, HasKeyTest) { @@ -600,17 +615,25 @@ TEST_F(PropertiesTestSuite, SetEntryTest) { auto* version = celix_version_create(1, 2, 3, nullptr); celix_properties_setVersion(props1, "key5", version); celix_version_destroy(version); + celix_autoptr(celix_array_list_t) longList = celix_arrayList_createLongArray(); + celix_arrayList_addLong(longList, 1); + celix_arrayList_addLong(longList, 2); + celix_arrayList_addLong(longList, 3); + celix_properties_setArrayList(props1, "key6", longList); CELIX_PROPERTIES_ITERATE(props1, visit) { celix_properties_setEntry(props2, visit.key, &visit.entry); + celix_properties_setEntry(nullptr, visit.key, &visit.entry); } - EXPECT_EQ(5, celix_properties_size(props2)); + EXPECT_EQ(6, celix_properties_size(props2)); EXPECT_STREQ("value1", celix_properties_getAsString(props2, "key1", nullptr)); EXPECT_EQ(123, celix_properties_getAsLong(props2, "key2", -1L)); EXPECT_EQ(true, celix_properties_getAsBool(props2, "key3", false)); EXPECT_EQ(3.14, celix_properties_getAsDouble(props2, "key4", -1.0)); EXPECT_EQ(CELIX_PROPERTIES_VALUE_TYPE_VERSION, celix_properties_getType(props2, "key5")); + EXPECT_EQ(CELIX_PROPERTIES_VALUE_TYPE_ARRAY_LIST, celix_properties_getType(props2, "key6")); + EXPECT_TRUE(celix_properties_equals(props1, props2)); celix_properties_destroy(props1); celix_properties_destroy(props2); @@ -708,6 +731,22 @@ TEST_F(PropertiesTestSuite, InvalidArgumentsTest) { EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, celix_properties_assign(props, nullptr, strdup("value"))); EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, celix_properties_assign(props, strdup("key"), nullptr)); EXPECT_EQ(2, celix_err_getErrorCount()); + + EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, celix_properties_assignString(props, nullptr, strdup("value"))); + + celix_autoptr(celix_array_list_t) list1 = celix_arrayList_create(); + celix_autoptr(celix_array_list_t) list2 = celix_arrayList_createStringArray(); + celix_autoptr(celix_array_list_t) list3 = celix_arrayList_createPointerArray(); + + EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, celix_properties_setArrayList(props, nullptr, list2)); + EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, celix_properties_setArrayList(props, "list", nullptr)); + EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, celix_properties_setArrayList(props, "list", list1)); + EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, celix_properties_setArrayList(props, "list", list3)); + + EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, celix_properties_assignArrayList(props, nullptr, celix_steal_ptr(list2))); + EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, celix_properties_assignArrayList(props, "list", nullptr)); + EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, celix_properties_assignArrayList(props, "list", celix_steal_ptr(list1))); + EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, celix_properties_assignArrayList(props, "list", celix_steal_ptr(list3))); } TEST_F(PropertiesTestSuite, GetStatsTest) { @@ -867,6 +906,7 @@ TEST_F(PropertiesTestSuite, GetTypedArrayListTest) { celix_properties_assignArrayList(props, "doubleList", doubleList); celix_properties_assignArrayList(props, "boolList", boolList); celix_properties_assignArrayList(props, "versionList", versionList); + celix_properties_setString(props, "string", "Hello world"); //When the celix_properties_getAsArrayList is called with the array lists celix_autoptr(celix_array_list_t) retrievedStringList = nullptr; @@ -887,8 +927,7 @@ TEST_F(PropertiesTestSuite, GetTypedArrayListTest) { EXPECT_TRUE(celix_arrayList_equals(boolList, retrievedBoolList)); EXPECT_TRUE(celix_arrayList_equals(versionList, retrievedVersionList)); - - ///When the celix_properties_getArrayList is called with the array lists + //When the celix_properties_getArrayList is called with the array lists const auto* retrievedStringList2 = celix_properties_getStringArrayList(props, "stringList"); const auto* retrievedLongList2 = celix_properties_getLongArrayList(props, "longList"); const auto* retrievedDoubleList2 = celix_properties_getDoubleArrayList(props, "doubleList"); @@ -908,6 +947,7 @@ TEST_F(PropertiesTestSuite, GetTypedArrayListTest) { const auto* retrievedDoubleList3 = celix_properties_getArrayList(props, "doubleList"); const auto* retrievedBoolList3 = celix_properties_getArrayList(props, "boolList"); const auto* retrievedVersionList3 = celix_properties_getArrayList(props, "versionList"); + EXPECT_EQ(nullptr, celix_properties_getArrayList(props, "missing")); //Then the retrieved array lists should be the same as the original array lists EXPECT_TRUE(celix_arrayList_equals(stringList, retrievedStringList3)); @@ -915,6 +955,12 @@ TEST_F(PropertiesTestSuite, GetTypedArrayListTest) { EXPECT_TRUE(celix_arrayList_equals(doubleList, retrievedDoubleList3)); EXPECT_TRUE(celix_arrayList_equals(boolList, retrievedBoolList3)); EXPECT_TRUE(celix_arrayList_equals(versionList, retrievedVersionList3)); + + celix_autoptr(celix_array_list_t) longList4 = celix_arrayList_copy(longList); + celix_autoptr(celix_array_list_t) retrievedLongList4 = nullptr; + EXPECT_EQ(CELIX_SUCCESS, celix_properties_getAsLongArrayList(props, "string", longList4, &retrievedLongList4)); + EXPECT_TRUE(celix_arrayList_equals(longList, retrievedLongList4)); + } TEST_F(PropertiesTestSuite, GetAsTypedArrayListWithInvalidDefault) { diff --git a/libs/utils/include/celix_properties.h b/libs/utils/include/celix_properties.h index 39e87c660..7c7a3e913 100644 --- a/libs/utils/include/celix_properties.h +++ b/libs/utils/include/celix_properties.h @@ -526,8 +526,7 @@ CELIX_UTILS_EXPORT celix_status_t celix_properties_getAsVersion(const celix_prop * * This function will make a copy of the provided celix_array_list_t object, using the celix_arrayList_copy function. * - * If an error occurs, the error status is returned and a message is logged to - * celix_err. + * If an error occurs, the error status is returned and a message is logged to celix_err. * * @param[in] properties The property set to modify. * @param[in] key The key of the property to set. @@ -556,7 +555,7 @@ CELIX_UTILS_EXPORT celix_status_t celix_properties_setArrayList(celix_properties * to the properties set. Cannot be NULL. * @return CELIX_SUCCESS if the operation was successful, CELIX_ENOMEM if there was not enough memory to set the entry, * and CELIX_ILLEGAL_ARGUMENT if the provided key is NULL, values is NULL or the array list type is - * valid. On error, the provided array list is destroyed. + * invalid. On error, the provided array list is destroyed. */ CELIX_UTILS_EXPORT celix_status_t celix_properties_assignArrayList(celix_properties_t* properties, const char* key, @@ -616,7 +615,7 @@ CELIX_UTILS_EXPORT celix_status_t celix_properties_getAsLongArrayList(const celi /** * @brief Get the property value as an array of longs without copying. * - * This function provides a non-owning, read-only access to a array of longs property value. + * This function provides a non-owning, read-only access to an array of longs property value. * It returns a const pointer to the array. If the property is not set or its value is not an array of longs, * NULL is returned. * @@ -658,7 +657,7 @@ CELIX_UTILS_EXPORT celix_status_t celix_properties_getAsDoubleArrayList(const ce /** * @brief Get the property value as an array of doubles without copying. * - * This function provides a non-owning, read-only access to a array of doubles property value. + * This function provides a non-owning, read-only access to an array of doubles property value. * It returns a const pointer to the array. If the property is not set or its value is not an array of doubles, * NULL is returned. * @@ -748,7 +747,7 @@ CELIX_UTILS_EXPORT celix_status_t celix_properties_getAsStringArrayList(const ce /** * @brief Get the property value as an array of strings without copying. * - * This function provides a non-owning, read-only access to a array of string property value. + * This function provides a non-owning, read-only access to an array of string property value. * It returns a const pointer to the array. If the property is not set or its value is not an array of strings, * NULL is returned. * @@ -796,7 +795,7 @@ CELIX_UTILS_EXPORT celix_status_t celix_properties_getAsVersionArrayList(const c /** * @brief Get the property value as an array of celix_version_t entries without copying. * - * This function provides a non-owning, read-only access to a array of celix_version_t property value. + * This function provides a non-owning, read-only access to an array of celix_version_t property value. * entries. It returns a const pointer to the array. If the property is not set or its value is not an array of * celix_version_t entries, NULL is returned. * diff --git a/libs/utils/src/properties.c b/libs/utils/src/properties.c index c51001127..e00573a76 100644 --- a/libs/utils/src/properties.c +++ b/libs/utils/src/properties.c @@ -603,18 +603,7 @@ celix_properties_t* celix_properties_copy(const celix_properties_t* properties) CELIX_PROPERTIES_ITERATE(properties, iter) { celix_status_t status; - if (iter.entry.valueType == CELIX_PROPERTIES_VALUE_TYPE_STRING) { - status = celix_properties_setString(copy, iter.key, iter.entry.value); - } else if (iter.entry.valueType == CELIX_PROPERTIES_VALUE_TYPE_LONG) { - status = celix_properties_setLong(copy, iter.key, iter.entry.typed.longValue); - } else if (iter.entry.valueType == CELIX_PROPERTIES_VALUE_TYPE_DOUBLE) { - status = celix_properties_setDouble(copy, iter.key, iter.entry.typed.doubleValue); - } else if (iter.entry.valueType == CELIX_PROPERTIES_VALUE_TYPE_BOOL) { - status = celix_properties_setBool(copy, iter.key, iter.entry.typed.boolValue); - } else /*version*/ { - assert(iter.entry.valueType == CELIX_PROPERTIES_VALUE_TYPE_VERSION); - status = celix_properties_setVersion(copy, iter.key, iter.entry.typed.versionValue); - } + status = celix_properties_setEntry(copy, iter.key, &iter.entry); if (status != CELIX_SUCCESS) { celix_err_pushf("Failed to copy property %s", iter.key); celix_properties_destroy(copy); @@ -691,14 +680,19 @@ celix_status_t celix_properties_assign(celix_properties_t* properties, char* key free(key); } return status; + } else { + free(key); + free(value); + return CELIX_SUCCESS; // silently ignore NULL properties } - return CELIX_SUCCESS; // silently ignore NULL properties, key or value } celix_status_t celix_properties_setEntry(celix_properties_t* properties, const char* key, const celix_properties_entry_t* entry) { if (entry) { switch (entry->valueType) { + case CELIX_PROPERTIES_VALUE_TYPE_STRING: + return celix_properties_setString(properties, key, entry->typed.strValue); case CELIX_PROPERTIES_VALUE_TYPE_LONG: return celix_properties_setLong(properties, key, entry->typed.longValue); case CELIX_PROPERTIES_VALUE_TYPE_DOUBLE: @@ -707,8 +701,8 @@ celix_properties_setEntry(celix_properties_t* properties, const char* key, const return celix_properties_setBool(properties, key, entry->typed.boolValue); case CELIX_PROPERTIES_VALUE_TYPE_VERSION: return celix_properties_setVersion(properties, key, entry->typed.versionValue); - default: // STRING - return celix_properties_set(properties, key, entry->typed.strValue); + default: //CELIX_PROPERTIES_VALUE_TYPE_ARRAY_LIST + return celix_properties_setArrayList(properties, key, entry->typed.arrayValue); } } return CELIX_SUCCESS; // silently ignore NULL entry @@ -721,6 +715,8 @@ static bool celix_properties_entryEquals(const celix_properties_entry_t* entry1, } switch (entry1->valueType) { + case CELIX_PROPERTIES_VALUE_TYPE_STRING: + return strcmp(entry1->value, entry2->value) == 0; case CELIX_PROPERTIES_VALUE_TYPE_LONG: return entry1->typed.longValue == entry2->typed.longValue; case CELIX_PROPERTIES_VALUE_TYPE_DOUBLE: @@ -729,8 +725,8 @@ static bool celix_properties_entryEquals(const celix_properties_entry_t* entry1, return entry1->typed.boolValue == entry2->typed.boolValue; case CELIX_PROPERTIES_VALUE_TYPE_VERSION: return celix_version_compareTo(entry1->typed.versionValue, entry2->typed.versionValue) == 0; - default: // STRING - return strcmp(entry1->value, entry2->value) == 0; + default: //CELIX_PROPERTIES_VALUE_TYPE_ARRAY_LIST + return celix_arrayList_equals(entry1->typed.arrayValue, entry2->typed.arrayValue); } } @@ -924,11 +920,10 @@ celix_properties_assignVersion(celix_properties_t* properties, const char* key, celix_status_t celix_properties_setArrayList(celix_properties_t* properties, const char* key, const celix_array_list_t* values) { - if (!key || !values) { + if (!key || !values || celix_arrayList_getElementType(values) == CELIX_ARRAY_LIST_ELEMENT_TYPE_UNDEFINED || + celix_arrayList_getElementType(values) == CELIX_ARRAY_LIST_ELEMENT_TYPE_POINTER) { return CELIX_ILLEGAL_ARGUMENT; } - assert(celix_arrayList_getElementType(values) != CELIX_ARRAY_LIST_ELEMENT_TYPE_UNDEFINED && - celix_arrayList_getElementType(values) != CELIX_ARRAY_LIST_ELEMENT_TYPE_POINTER); //wrong array list type celix_array_list_t* copy = celix_arrayList_copy(values); if (!copy) { return CELIX_ENOMEM; @@ -941,12 +936,11 @@ celix_properties_setArrayList(celix_properties_t* properties, const char* key, c celix_status_t celix_properties_assignArrayList(celix_properties_t* properties, const char* key, celix_array_list_t* values) { - if (!key || !values) { + if (!key || !values || celix_arrayList_getElementType(values) == CELIX_ARRAY_LIST_ELEMENT_TYPE_UNDEFINED || + celix_arrayList_getElementType(values) == CELIX_ARRAY_LIST_ELEMENT_TYPE_POINTER) { celix_arrayList_destroy(values); return CELIX_ILLEGAL_ARGUMENT; } - assert(celix_arrayList_getElementType(values) != CELIX_ARRAY_LIST_ELEMENT_TYPE_UNDEFINED && - celix_arrayList_getElementType(values) != CELIX_ARRAY_LIST_ELEMENT_TYPE_POINTER); //wrong array list type celix_properties_entry_t prototype = {0}; prototype.valueType = CELIX_PROPERTIES_VALUE_TYPE_ARRAY_LIST; prototype.typed.arrayValue = values; @@ -1085,9 +1079,6 @@ bool celix_properties_equals(const celix_properties_t* props1, const celix_prope if (props1 == props2) { return true; } - if (props1 == NULL && props2 == NULL) { - return true; - } if (props1 == NULL || props2 == NULL) { return false; } From d38f0b10ac60d2936a5676e483c8940d5c5e4f50 Mon Sep 17 00:00:00 2001 From: PengZheng Date: Tue, 2 Apr 2024 21:43:14 +0800 Subject: [PATCH 2/3] gh-674: Add missing test for celix_properties_setEntry. --- libs/utils/gtest/src/PropertiesTestSuite.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/utils/gtest/src/PropertiesTestSuite.cc b/libs/utils/gtest/src/PropertiesTestSuite.cc index 7e15b0557..90636b79e 100644 --- a/libs/utils/gtest/src/PropertiesTestSuite.cc +++ b/libs/utils/gtest/src/PropertiesTestSuite.cc @@ -625,7 +625,7 @@ TEST_F(PropertiesTestSuite, SetEntryTest) { celix_properties_setEntry(props2, visit.key, &visit.entry); celix_properties_setEntry(nullptr, visit.key, &visit.entry); } - + celix_properties_setEntry(props2, "key7", nullptr); EXPECT_EQ(6, celix_properties_size(props2)); EXPECT_STREQ("value1", celix_properties_getAsString(props2, "key1", nullptr)); EXPECT_EQ(123, celix_properties_getAsLong(props2, "key2", -1L)); From 6a9713e0553ee536e010aed455c2205eae380bfd Mon Sep 17 00:00:00 2001 From: Pepijn Noltes Date: Tue, 2 Apr 2024 20:28:08 +0200 Subject: [PATCH 3/3] gh-674: Rename array el type enum to string values --- libs/utils/gtest/src/ArrayListTestSuite.cc | 21 ++++++++------------- libs/utils/src/array_list.c | 14 +++++++------- 2 files changed, 15 insertions(+), 20 deletions(-) diff --git a/libs/utils/gtest/src/ArrayListTestSuite.cc b/libs/utils/gtest/src/ArrayListTestSuite.cc index 7af65ac5a..c9219b9fb 100644 --- a/libs/utils/gtest/src/ArrayListTestSuite.cc +++ b/libs/utils/gtest/src/ArrayListTestSuite.cc @@ -497,18 +497,13 @@ TEST_F(ArrayListTestSuite, ReallocTest) { } TEST_F(ArrayListTestSuite, ElementTypeToStringTest) { - EXPECT_STREQ("CELIX_ARRAY_LIST_ELEMENT_TYPE_UNDEFINED", - celix_arrayList_elementTypeToString(CELIX_ARRAY_LIST_ELEMENT_TYPE_UNDEFINED)); - EXPECT_STREQ("CELIX_ARRAY_LIST_ELEMENT_TYPE_POINTER", - celix_arrayList_elementTypeToString(CELIX_ARRAY_LIST_ELEMENT_TYPE_POINTER)); - EXPECT_STREQ("CELIX_ARRAY_LIST_ELEMENT_TYPE_STRING", - celix_arrayList_elementTypeToString(CELIX_ARRAY_LIST_ELEMENT_TYPE_STRING)); - EXPECT_STREQ("CELIX_ARRAY_LIST_ELEMENT_TYPE_LONG", - celix_arrayList_elementTypeToString(CELIX_ARRAY_LIST_ELEMENT_TYPE_LONG)); - EXPECT_STREQ("CELIX_ARRAY_LIST_ELEMENT_TYPE_DOUBLE", - celix_arrayList_elementTypeToString(CELIX_ARRAY_LIST_ELEMENT_TYPE_DOUBLE)); - EXPECT_STREQ("CELIX_ARRAY_LIST_ELEMENT_TYPE_BOOL", - celix_arrayList_elementTypeToString(CELIX_ARRAY_LIST_ELEMENT_TYPE_BOOL)); - EXPECT_STREQ("CELIX_ARRAY_LIST_ELEMENT_TYPE_UNDEFINED", + EXPECT_STREQ("Undefined", celix_arrayList_elementTypeToString(CELIX_ARRAY_LIST_ELEMENT_TYPE_UNDEFINED)); + EXPECT_STREQ("Pointer", celix_arrayList_elementTypeToString(CELIX_ARRAY_LIST_ELEMENT_TYPE_POINTER)); + EXPECT_STREQ("String", celix_arrayList_elementTypeToString(CELIX_ARRAY_LIST_ELEMENT_TYPE_STRING)); + EXPECT_STREQ("Long", celix_arrayList_elementTypeToString(CELIX_ARRAY_LIST_ELEMENT_TYPE_LONG)); + EXPECT_STREQ("Double", celix_arrayList_elementTypeToString(CELIX_ARRAY_LIST_ELEMENT_TYPE_DOUBLE)); + EXPECT_STREQ("Bool", celix_arrayList_elementTypeToString(CELIX_ARRAY_LIST_ELEMENT_TYPE_BOOL)); + EXPECT_STREQ("Version", celix_arrayList_elementTypeToString(CELIX_ARRAY_LIST_ELEMENT_TYPE_VERSION)); + EXPECT_STREQ("Undefined", celix_arrayList_elementTypeToString((celix_array_list_element_type_t)100 /*non existing*/)); } diff --git a/libs/utils/src/array_list.c b/libs/utils/src/array_list.c index f4cdcd36c..361525f2e 100644 --- a/libs/utils/src/array_list.c +++ b/libs/utils/src/array_list.c @@ -28,13 +28,13 @@ #include "celix_utils.h" #include "celix_version.h" -#define STRING_VALUE_UNDEFINED_EL_TYPE "CELIX_ARRAY_LIST_ELEMENT_TYPE_UNDEFINED" -#define STRING_VALUE_POINTER_EL_TYPE "CELIX_ARRAY_LIST_ELEMENT_TYPE_POINTER" -#define STRING_VALUE_STRING_EL_TYPE "CELIX_ARRAY_LIST_ELEMENT_TYPE_STRING" -#define STRING_VALUE_LONG_EL_TYPE "CELIX_ARRAY_LIST_ELEMENT_TYPE_LONG" -#define STRING_VALUE_DOUBLE_EL_TYPE "CELIX_ARRAY_LIST_ELEMENT_TYPE_DOUBLE" -#define STRING_VALUE_BOOL_EL_TYPE "CELIX_ARRAY_LIST_ELEMENT_TYPE_BOOL" -#define STRING_VALUE_VERSION_EL_TYPE "CELIX_ARRAY_LIST_ELEMENT_TYPE_VERSION" +#define STRING_VALUE_UNDEFINED_EL_TYPE "Undefined" +#define STRING_VALUE_POINTER_EL_TYPE "Pointer" +#define STRING_VALUE_STRING_EL_TYPE "String" +#define STRING_VALUE_LONG_EL_TYPE "Long" +#define STRING_VALUE_DOUBLE_EL_TYPE "Double" +#define STRING_VALUE_BOOL_EL_TYPE "Bool" +#define STRING_VALUE_VERSION_EL_TYPE "Version" struct celix_array_list { celix_array_list_element_type_t elementType;