This is an automated email from the ASF dual-hosted git repository. pnoltes pushed a commit to branch feature/type_support_for_properties in repository https://gitbox.apache.org/repos/asf/celix.git
commit ced3279168aae20863106951921b3b8e02ecea28 Author: Pepijn Noltes <[email protected]> AuthorDate: Sun Jan 8 14:58:10 2023 +0100 Refactor and improve properties and string hash map test suite and impl --- libs/framework/include/celix/Exception.h | 4 + libs/utils/gtest/CMakeLists.txt | 1 + libs/utils/gtest/src/CxxPropertiesTestSuite.cc | 34 ++- libs/utils/gtest/src/CxxVersionTestSuite.cc | 93 ++++++++ libs/utils/gtest/src/HashMapTestSuite.cc | 20 ++ libs/utils/gtest/src/PropertiesTestSuite.cc | 110 +++++----- libs/utils/gtest/src/VersionTestSuite.cc | 244 ++++++++------------- .../include/celix/IOException.h} | 21 +- libs/utils/include/celix/Properties.h | 70 +++++- libs/utils/include/celix/Version.h | 9 +- libs/utils/include/celix_long_hash_map.h | 8 + libs/utils/include/celix_properties.h | 14 +- libs/utils/include/celix_string_hash_map.h | 33 ++- libs/utils/src/celix_hash_map.c | 48 +++- libs/utils/src/properties.c | 51 +++-- libs/utils/src/version.c | 2 +- 16 files changed, 486 insertions(+), 276 deletions(-) diff --git a/libs/framework/include/celix/Exception.h b/libs/framework/include/celix/Exception.h index 1675567d..b7186634 100644 --- a/libs/framework/include/celix/Exception.h +++ b/libs/framework/include/celix/Exception.h @@ -27,7 +27,11 @@ namespace celix { */ class Exception : public std::exception { public: +#if __cplusplus >= 201703L //C++17 or higher + explicit Exception(std::string_view msg) : w{msg} {} +#else explicit Exception(std::string msg) : w{std::move(msg)} {} +#endif Exception(const Exception&) = default; Exception(Exception&&) = default; diff --git a/libs/utils/gtest/CMakeLists.txt b/libs/utils/gtest/CMakeLists.txt index 1e512fe6..894fba32 100644 --- a/libs/utils/gtest/CMakeLists.txt +++ b/libs/utils/gtest/CMakeLists.txt @@ -32,6 +32,7 @@ add_executable(test_utils src/FileUtilsTestSuite.cc src/PropertiesTestSuite.cc src/VersionTestSuite.cc + src/CxxVersionTestSuite.cc ${CELIX_UTIL_TEST_SOURCES_FOR_CXX_HEADERS} ) diff --git a/libs/utils/gtest/src/CxxPropertiesTestSuite.cc b/libs/utils/gtest/src/CxxPropertiesTestSuite.cc index 142570c4..b2f39192 100644 --- a/libs/utils/gtest/src/CxxPropertiesTestSuite.cc +++ b/libs/utils/gtest/src/CxxPropertiesTestSuite.cc @@ -27,12 +27,12 @@ class CxxPropertiesTestSuite : public ::testing::Test { public: }; -TEST_F(CxxPropertiesTestSuite, testCreateDestroy) { +TEST_F(CxxPropertiesTestSuite, CreateDestroyTest) { celix::Properties props{}; EXPECT_EQ(0, props.size()); } -TEST_F(CxxPropertiesTestSuite, testFillAndLoop) { +TEST_F(CxxPropertiesTestSuite, FillAndLoopTest) { celix::Properties props{}; EXPECT_EQ(0, props.size()); @@ -66,7 +66,7 @@ TEST_F(CxxPropertiesTestSuite, testFillAndLoop) { EXPECT_EQ(5, count); } -TEST_F(CxxPropertiesTestSuite, testLoopForSize0And1) { +TEST_F(CxxPropertiesTestSuite, LoopForSize0And1Test) { celix::Properties props0{}; for (const auto& pair : props0) { FAIL() << "Should not get an loop entry with a properties size of 0. got key: " << pair.first; @@ -82,7 +82,7 @@ TEST_F(CxxPropertiesTestSuite, testLoopForSize0And1) { EXPECT_EQ(1, count); } -TEST_F(CxxPropertiesTestSuite, testCopy) { +TEST_F(CxxPropertiesTestSuite, CopyTest) { celix::Properties props{}; props["key1"] = "value1"; @@ -97,7 +97,7 @@ TEST_F(CxxPropertiesTestSuite, testCopy) { EXPECT_EQ(v2, "value1_new"); } -TEST_F(CxxPropertiesTestSuite, testWrap) { +TEST_F(CxxPropertiesTestSuite, WrapTest) { auto *props = celix_properties_create(); celix_properties_set(props, "test", "test"); @@ -112,7 +112,7 @@ TEST_F(CxxPropertiesTestSuite, testWrap) { celix_properties_destroy(props); } -TEST_F(CxxPropertiesTestSuite, getType) { +TEST_F(CxxPropertiesTestSuite, GetTypeTest) { celix::Properties props{}; props.set("bool", true); @@ -140,7 +140,7 @@ TEST_F(CxxPropertiesTestSuite, getType) { EXPECT_EQ(props.getType("version"), celix::Properties::ValueType::Version); } -TEST_F(CxxPropertiesTestSuite, testGetAsVersion) { +TEST_F(CxxPropertiesTestSuite, GetAsVersionTest) { celix::Properties props; // Test getting a version from a string property @@ -163,8 +163,24 @@ TEST_F(CxxPropertiesTestSuite, testGetAsVersion) { EXPECT_EQ(props.getAsVersion("key", celix::Version{4, 5, 6}), ver); } +TEST_F(CxxPropertiesTestSuite, StoreAndLoadTest) { + std::string path{"cxx_store_and_load_test.properties"}; + + celix::Properties props{}; + props.set("key1", 1); + props.set("key2", 2); + + EXPECT_NO_THROW(props.store(path)); + + celix::Properties loadedProps{}; + EXPECT_NO_THROW(loadedProps = celix::Properties::load(path)); + EXPECT_EQ(props.size(), loadedProps.size()); + + EXPECT_THROW(loadedProps = celix::Properties::load("non-existence"), celix::IOException); +} + #if __cplusplus >= 201703L //C++17 or higher -TEST_F(CxxPropertiesTestSuite, testStringView) { +TEST_F(CxxPropertiesTestSuite, StringViewTest) { constexpr std::string_view stringViewKey = "KEY1"; constexpr std::string_view stringViewValue = "VALUE1"; std::string stringKey{"KEY2"}; @@ -219,7 +235,7 @@ TEST_F(CxxPropertiesTestSuite, testStringView) { } } -TEST_F(CxxPropertiesTestSuite, testUseOfConstexprInSetMethod) { +TEST_F(CxxPropertiesTestSuite, UseOfConstexprInSetMethodTest) { celix::Properties props{}; //Test if different bool "types" are correctly handled diff --git a/libs/utils/gtest/src/CxxVersionTestSuite.cc b/libs/utils/gtest/src/CxxVersionTestSuite.cc new file mode 100644 index 00000000..0055d5c4 --- /dev/null +++ b/libs/utils/gtest/src/CxxVersionTestSuite.cc @@ -0,0 +1,93 @@ +/* + * 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/Version.h" + +class CxxVersionTestSuite : public ::testing::Test {}; + +#include "gtest/gtest.h" +#include "celix/Version.h" + +TEST_F(CxxVersionTestSuite, DefaultConstructorTest) { + celix::Version v; + EXPECT_EQ(0, v.getMajor()); + EXPECT_EQ(0, v.getMinor()); + EXPECT_EQ(0, v.getMicro()); + EXPECT_EQ("", v.getQualifier()); +} + +TEST_F(CxxVersionTestSuite, ConstructorTest) { + celix::Version v1{1, 2, 3, "qualifier"}; + EXPECT_EQ(1, v1.getMajor()); + EXPECT_EQ(2, v1.getMinor()); + EXPECT_EQ(3, v1.getMicro()); + EXPECT_EQ("qualifier", v1.getQualifier()); + + #if __cplusplus >= 201703L //C++17 or higher + celix::Version v2{1, 2, 3, std::string_view{"qualifier"}}; + EXPECT_EQ(1, v2.getMajor()); + EXPECT_EQ(2, v2.getMinor()); + EXPECT_EQ(3, v2.getMicro()); + EXPECT_EQ("qualifier", v2.getQualifier()); + #endif +} + +TEST_F(CxxVersionTestSuite, MoveConstructorTest) { + celix::Version v1{2, 3, 4, "qualifier"}; + celix::Version v2{std::move(v1)}; + + //v2 should have the values of v1 before the move + EXPECT_EQ(v2.getMajor(), 2); + EXPECT_EQ(v2.getMinor(), 3); + EXPECT_EQ(v2.getMicro(), 4); + EXPECT_EQ(v2.getQualifier(), "qualifier"); +} + +TEST_F(CxxVersionTestSuite, CopyConstructorTest) { + celix::Version v1{1, 2, 3, "qualifier"}; + celix::Version v2 = v1; + + EXPECT_EQ(v1, v2); + EXPECT_EQ(v1.getMajor(), v2.getMajor()); + EXPECT_EQ(v1.getMinor(), v2.getMinor()); + EXPECT_EQ(v1.getMicro(), v2.getMicro()); + EXPECT_EQ(v1.getQualifier(), v2.getQualifier()); +} + +TEST_F(CxxVersionTestSuite, MapKeyTest) { + // Test using Version as key in std::map + std::map<celix::Version, int> map; + map[celix::Version{1, 2, 3}] = 1; + map[celix::Version{3, 2, 1}] = 2; + + EXPECT_EQ(map[(celix::Version{1, 2, 3})], 1); + EXPECT_EQ(map[(celix::Version{3, 2, 1})], 2); +} + +TEST_F(CxxVersionTestSuite, UnorderedMapKeyTest) { + // Test using Version as key in std::unordered_map + std::unordered_map<celix::Version, int> unorderedMap; + unorderedMap[celix::Version{1, 2, 3}] = 1; + unorderedMap[celix::Version{3, 2, 1}] = 2; + + EXPECT_EQ(unorderedMap[(celix::Version{1, 2, 3})], 1); + EXPECT_EQ(unorderedMap[(celix::Version{3, 2, 1})], 2); +} diff --git a/libs/utils/gtest/src/HashMapTestSuite.cc b/libs/utils/gtest/src/HashMapTestSuite.cc index 27f0589a..7f9010e3 100644 --- a/libs/utils/gtest/src/HashMapTestSuite.cc +++ b/libs/utils/gtest/src/HashMapTestSuite.cc @@ -632,3 +632,23 @@ TEST_F(HashMapTestSuite, EqualsZeroSizeMapTest) { celix_stringHashMap_destroy(sMap); celix_longHashMap_destroy(lMap); } + +TEST_F(HashMapTestSuite, StoreKeysWeaklyTest) { + celix_string_hash_map_create_options_t opts{}; + opts.removedCallbackData = (void*)0x1; + opts.storeKeysWeakly = true; + opts.removedKeyCallback = [](void* data, char* key) { + EXPECT_EQ(data, (void*)0x1); + free(key); + }; + auto* sMap = celix_stringHashMap_createWithOptions(&opts); + EXPECT_FALSE(celix_stringHashMap_putLong(sMap, celix_utils_strdup("key1"), 1)); //new key -> takes ownership + EXPECT_TRUE(celix_stringHashMap_putLong(sMap, "key1", 2)); //replace key -> takes no ownership + + EXPECT_FALSE(celix_stringHashMap_putLong(sMap, celix_utils_strdup("key2"), 3)); //new key -> takes ownership + EXPECT_TRUE(celix_stringHashMap_putLong(sMap, "key2", 4)); //replace key -> takes no ownership + celix_stringHashMap_remove(sMap, "key1"); + + celix_stringHashMap_destroy(sMap); +} + diff --git a/libs/utils/gtest/src/PropertiesTestSuite.cc b/libs/utils/gtest/src/PropertiesTestSuite.cc index 74e96f6e..f21b4049 100644 --- a/libs/utils/gtest/src/PropertiesTestSuite.cc +++ b/libs/utils/gtest/src/PropertiesTestSuite.cc @@ -20,6 +20,7 @@ #include <gtest/gtest.h> #include "celix_properties.h" +#include "celix_utils.h" using ::testing::MatchesRegex; @@ -28,14 +29,14 @@ public: }; -TEST_F(PropertiesTestSuite, create) { +TEST_F(PropertiesTestSuite, CreateTest) { auto* properties = celix_properties_create(); EXPECT_TRUE(properties); celix_properties_destroy(properties); } -TEST_F(PropertiesTestSuite, load) { +TEST_F(PropertiesTestSuite, LoadTest) { char propertiesFile[] = "resources-test/properties.txt"; auto* properties = celix_properties_load(propertiesFile); EXPECT_EQ(4, celix_properties_size(properties)); @@ -55,7 +56,21 @@ TEST_F(PropertiesTestSuite, load) { celix_properties_destroy(properties); } -TEST_F(PropertiesTestSuite, asLong) { +TEST_F(PropertiesTestSuite, StoreTest) { + const char* propertiesFile = "resources-test/properties_out.txt"; + auto* properties = celix_properties_create(); + char keyA[] = "x"; + char keyB[] = "y"; + char valueA[] = "1"; + char valueB[] = "2"; + celix_properties_set(properties, keyA, valueA); + celix_properties_set(properties, keyB, valueB); + celix_properties_store(properties, propertiesFile, nullptr); + + celix_properties_destroy(properties); +} + +TEST_F(PropertiesTestSuite, GetAsLongTest) { celix_properties_t *props = celix_properties_create(); celix_properties_set(props, "t1", "42"); celix_properties_set(props, "t2", "-42"); @@ -84,21 +99,7 @@ TEST_F(PropertiesTestSuite, asLong) { celix_properties_destroy(props); } -TEST_F(PropertiesTestSuite, store) { - char propertiesFile[] = "resources-test/properties_out.txt"; - auto* properties = celix_properties_create(); - char keyA[] = "x"; - char keyB[] = "y"; - char valueA[] = "1"; - char valueB[] = "2"; - celix_properties_set(properties, keyA, valueA); - celix_properties_set(properties, keyB, valueB); - celix_properties_store(properties, propertiesFile, nullptr); - - celix_properties_destroy(properties); -} - -TEST_F(PropertiesTestSuite, copy) { +TEST_F(PropertiesTestSuite, CopyTest) { char propertiesFile[] = "resources-test/properties.txt"; auto* properties = celix_properties_load(propertiesFile); EXPECT_EQ(4, celix_properties_size(properties)); @@ -115,7 +116,7 @@ TEST_F(PropertiesTestSuite, copy) { celix_properties_destroy(copy); } -TEST_F(PropertiesTestSuite, getSet) { +TEST_F(PropertiesTestSuite, GetSetTest) { auto* properties = celix_properties_create(); char keyA[] = "x"; char keyB[] = "y"; @@ -137,7 +138,7 @@ TEST_F(PropertiesTestSuite, getSet) { celix_properties_destroy(properties); } -TEST_F(PropertiesTestSuite, getSetWithNULL) { +TEST_F(PropertiesTestSuite, GetSetWithNullTest) { auto* properties = celix_properties_create(); celix_properties_set(properties, nullptr, "value"); @@ -153,7 +154,7 @@ TEST_F(PropertiesTestSuite, getSetWithNULL) { } -TEST_F(PropertiesTestSuite, setUnset) { +TEST_F(PropertiesTestSuite, SetUnsetTest) { auto* properties = celix_properties_create(); char keyA[] = "x"; char *keyD = strndup("a", 1); @@ -171,7 +172,7 @@ TEST_F(PropertiesTestSuite, setUnset) { celix_properties_destroy(properties); } -TEST_F(PropertiesTestSuite, longTest) { +TEST_F(PropertiesTestSuite, GetLongTest) { auto* properties = celix_properties_create(); celix_properties_set(properties, "a", "2"); @@ -201,7 +202,7 @@ TEST_F(PropertiesTestSuite, longTest) { celix_properties_destroy(properties); } -TEST_F(PropertiesTestSuite, boolTest) { +TEST_F(PropertiesTestSuite, GetBoolTest) { auto* properties = celix_properties_create(); celix_properties_set(properties, "a", "true"); @@ -233,7 +234,7 @@ TEST_F(PropertiesTestSuite, boolTest) { celix_properties_destroy(properties); } -TEST_F(PropertiesTestSuite, fillTest) { +TEST_F(PropertiesTestSuite, GetFillTest) { celix_properties_t *props = celix_properties_create(); int testCount = 1000; for (int i = 0; i < 1000; ++i) { @@ -246,31 +247,34 @@ TEST_F(PropertiesTestSuite, fillTest) { celix_properties_destroy(props); } -TEST_F(PropertiesTestSuite, setOverwrite) { +TEST_F(PropertiesTestSuite, GetSetOverwrite) { auto* props = celix_properties_create(); auto* version = celix_version_createEmptyVersion(); - const char* key = "key"; - - celix_properties_set(props, key, "str1"); - EXPECT_STREQ("str1", celix_properties_get(props, key, "")); - celix_properties_set(props, key, "str2"); - EXPECT_STREQ("str2", celix_properties_get(props, key, "")); - celix_properties_setLong(props, key, 1); - EXPECT_EQ(1, celix_properties_getAsLong(props, key, -1L)); - celix_properties_setDouble(props, key, 2.0); - EXPECT_EQ(2.0, celix_properties_getAsLong(props, key, -2.0)); - celix_properties_setBool(props, key, false); - EXPECT_EQ(false, celix_properties_getAsBool(props, key, true)); - celix_properties_setVersionWithoutCopy(props, key, version); - EXPECT_EQ(version, celix_properties_getVersion(props, key, nullptr)); - celix_properties_set(props, key, "last"); + + { + char* key = celix_utils_strdup("key"); + celix_properties_set(props, key, "str1"); + free(key); + } + EXPECT_STREQ("str1", celix_properties_get(props, "key", "")); + celix_properties_set(props, "key", "str2"); + EXPECT_STREQ("str2", celix_properties_get(props, "key", "")); + celix_properties_setLong(props, "key", 1); + EXPECT_EQ(1, celix_properties_getAsLong(props, "key", -1L)); + celix_properties_setDouble(props, "key", 2.0); + EXPECT_EQ(2.0, celix_properties_getAsLong(props, "key", -2.0)); + celix_properties_setBool(props, "key", false); + EXPECT_EQ(false, celix_properties_getAsBool(props, "key", true)); + celix_properties_setVersionWithoutCopy(props, "key", version); + EXPECT_EQ(version, celix_properties_getVersion(props, "key", nullptr)); + celix_properties_set(props, "key", "last"); celix_properties_destroy(props); } -TEST_F(PropertiesTestSuite, sizeAndIteratorTest) { +TEST_F(PropertiesTestSuite, SizeAndIteratorTest) { celix_properties_t *props = celix_properties_create(); EXPECT_EQ(0, celix_properties_size(props)); celix_properties_set(props, "a", "1"); @@ -291,7 +295,7 @@ TEST_F(PropertiesTestSuite, sizeAndIteratorTest) { celix_properties_destroy(props); } -TEST_F(PropertiesTestSuite, getType) { +TEST_F(PropertiesTestSuite, GetTypeTest) { auto* props = celix_properties_create(); celix_properties_set(props, "string", "value"); celix_properties_setLong(props, "long", 123); @@ -311,7 +315,7 @@ TEST_F(PropertiesTestSuite, getType) { celix_properties_destroy(props); } -TEST_F(PropertiesTestSuite, getEntry) { +TEST_F(PropertiesTestSuite, GetEntryTest) { auto* props = celix_properties_create(); celix_properties_set(props, "key1", "value1"); celix_properties_setLong(props, "key2", 123); @@ -354,7 +358,7 @@ TEST_F(PropertiesTestSuite, getEntry) { celix_properties_destroy(props); } -TEST_F(PropertiesTestSuite, iteratorNextKey) { +TEST_F(PropertiesTestSuite, IteratorNextKeyTest) { auto* props = celix_properties_create(); celix_properties_set(props, "key1", "value1"); celix_properties_set(props, "key2", "value2"); @@ -374,7 +378,7 @@ TEST_F(PropertiesTestSuite, iteratorNextKey) { celix_properties_destroy(props); } -TEST_F(PropertiesTestSuite, iteratorNext) { +TEST_F(PropertiesTestSuite, IteratorNextTest) { auto* props = celix_properties_create(); celix_properties_set(props, "key1", "value1"); celix_properties_set(props, "key2", "value2"); @@ -383,7 +387,7 @@ TEST_F(PropertiesTestSuite, iteratorNext) { int count = 0; auto iter = celix_properties_begin(props); while (!celix_propertiesIterator_isEnd(&iter)) { - EXPECT_NE(strstr(iter.entry.key, "key"), nullptr); + EXPECT_NE(strstr(iter.key, "key"), nullptr); EXPECT_NE(strstr(iter.entry.value, "value"), nullptr); count++; celix_propertiesIterator_next(&iter); @@ -395,7 +399,7 @@ TEST_F(PropertiesTestSuite, iteratorNext) { celix_properties_destroy(props); } -TEST_F(PropertiesTestSuite, iterateOverProperties) { +TEST_F(PropertiesTestSuite, IterateOverPropertiesTest) { auto* props = celix_properties_create(); celix_properties_set(props, "key1", "value1"); celix_properties_set(props, "key2", "value2"); @@ -404,12 +408,12 @@ TEST_F(PropertiesTestSuite, iterateOverProperties) { int innerCount = 0; CELIX_PROPERTIES_ITERATE(props, outerIter) { outerCount++; - EXPECT_NE(strstr(outerIter.entry.key, "key"), nullptr); + EXPECT_NE(strstr(outerIter.key, "key"), nullptr); // Inner loop to test nested iteration CELIX_PROPERTIES_ITERATE(props, innerIter) { innerCount++; - EXPECT_NE(strstr(innerIter.entry.key, "key"), nullptr); + EXPECT_NE(strstr(innerIter.key, "key"), nullptr); } } // Check that both entries were iterated over @@ -428,7 +432,7 @@ TEST_F(PropertiesTestSuite, iterateOverProperties) { celix_properties_destroy(props); } -TEST_F(PropertiesTestSuite, getVersion) { +TEST_F(PropertiesTestSuite, GetVersionTest) { auto* properties = celix_properties_create(); auto* emptyVersion = celix_version_createEmptyVersion(); @@ -483,7 +487,7 @@ TEST_F(PropertiesTestSuite, getVersion) { celix_properties_destroy(properties); } -TEST_F(PropertiesTestSuite, TestEndOfProperties) { +TEST_F(PropertiesTestSuite, EndOfPropertiesTest) { auto* props = celix_properties_create(); celix_properties_set(props, "key1", "value1"); celix_properties_set(props, "key2", "value2"); @@ -495,7 +499,7 @@ TEST_F(PropertiesTestSuite, TestEndOfProperties) { celix_properties_destroy(props); } -TEST_F(PropertiesTestSuite, TestEndOfEmptyProperties) { +TEST_F(PropertiesTestSuite, EndOfEmptyPropertiesTest) { auto* props = celix_properties_create(); celix_properties_iterator_t endIter = celix_properties_end(props); @@ -504,3 +508,5 @@ TEST_F(PropertiesTestSuite, TestEndOfEmptyProperties) { celix_properties_destroy(props); } + +//TODO test replace and replace WithCopy \ No newline at end of file diff --git a/libs/utils/gtest/src/VersionTestSuite.cc b/libs/utils/gtest/src/VersionTestSuite.cc index 90e5e9f1..54539c7b 100644 --- a/libs/utils/gtest/src/VersionTestSuite.cc +++ b/libs/utils/gtest/src/VersionTestSuite.cc @@ -24,208 +24,134 @@ #include "celix_utils.h" #include "version.h" -extern "C" -{ -#include "version_private.h" -} - -class VersionTestSuite : public ::testing::Test {}; - -TEST_F(VersionTestSuite, create) { +class VersionTestSuite : public ::testing::Test { +public: + void expectVersion(const celix_version_t* version, int major, int minor, int micro, const char* qualifier = "") { + if (version) { + EXPECT_EQ(major, celix_version_getMajor(version)); + EXPECT_EQ(minor, celix_version_getMinor(version)); + EXPECT_EQ(micro, celix_version_getMicro(version)); + EXPECT_STREQ(qualifier, celix_version_getQualifier(version)); + } + } +}; + +TEST_F(VersionTestSuite, CreateTest) { celix_version_t* version = nullptr; - char * str; - str = celix_utils_strdup("abc"); - EXPECT_EQ(CELIX_SUCCESS, version_createVersion(1, 2, 3, str, &version)); + version = celix_version_create(1, 2, 3, "abc"); EXPECT_TRUE(version != nullptr); - EXPECT_EQ(1, version->major); - EXPECT_EQ(2, version->minor); - EXPECT_EQ(3, version->micro); - EXPECT_STREQ("abc", version->qualifier); + expectVersion(version, 1, 2, 3, "abc"); + celix_version_destroy(version); - version_destroy(version); + EXPECT_EQ(nullptr, celix_version_createVersion(-1, -2, -3, "abc")); + EXPECT_EQ(nullptr, celix_version_createVersion(-1, -2, -3, "abc|xyz")); + + + //Testing deprecated api version = nullptr; EXPECT_EQ(CELIX_SUCCESS, version_createVersion(1, 2, 3, nullptr, &version)); EXPECT_TRUE(version != nullptr); - EXPECT_EQ(1, version->major); - EXPECT_EQ(2, version->minor); - EXPECT_EQ(3, version->micro); - EXPECT_STREQ("", version->qualifier); - + int major; + int minor; + int micro; + const char* q; + EXPECT_EQ(CELIX_SUCCESS, version_getMajor(version, &major)); + EXPECT_EQ(CELIX_SUCCESS, version_getMinor(version, &minor)); + EXPECT_EQ(CELIX_SUCCESS, version_getMicro(version, µ)); + EXPECT_EQ(CELIX_SUCCESS, version_getQualifier(version, &q)); + EXPECT_EQ(1, major); + EXPECT_EQ(2, minor); + EXPECT_EQ(3, micro); + EXPECT_STREQ("", q); version_destroy(version); - version = nullptr; - free(str); - str = celix_utils_strdup("abc"); - EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, version_createVersion(-1, -2, -3, str, &version)); - version_destroy(version); version = nullptr; - free(str); - str = celix_utils_strdup("abc|xyz"); - EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, version_createVersion(1, 2, 3, str, &version)); - - version_destroy(version); - free(str); + EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, version_createVersion(-1, -2, -3, "abc", &version)); + EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, version_createVersion(1, 2, 3, "abc|xyz", &version)); } -TEST_F(VersionTestSuite, copy) { +TEST_F(VersionTestSuite, CopyTest) { auto* version = celix_version_create(1, 2, 3, "abc"); auto* copy = celix_version_copy(version); EXPECT_NE(nullptr, version); EXPECT_NE(nullptr, copy); - EXPECT_EQ(1, celix_version_getMajor(copy)); - EXPECT_EQ(2, celix_version_getMinor(copy)); - EXPECT_EQ(3, celix_version_getMicro(copy)); - EXPECT_STREQ("abc", celix_version_getQualifier(copy)); + expectVersion(version, 1, 2, 3, "abc"); celix_version_destroy(copy); celix_version_destroy(version); copy = celix_version_copy(nullptr); //returns "empty" version EXPECT_NE(nullptr, copy); - EXPECT_EQ(0, celix_version_getMajor(copy)); - EXPECT_EQ(0, celix_version_getMinor(copy)); - EXPECT_EQ(0, celix_version_getMicro(copy)); - EXPECT_STREQ("", celix_version_getQualifier(copy)); + expectVersion(copy, 0, 0, 0, ""); celix_version_destroy(copy); } -TEST_F(VersionTestSuite, createFromString) { - celix_version_t* version = nullptr; - celix_status_t status = CELIX_SUCCESS; - char * str; - - str = celix_utils_strdup("1"); - EXPECT_EQ(CELIX_SUCCESS, version_createVersionFromString(str, &version)); +TEST_F(VersionTestSuite, CreateFromStringTest) { + auto* version = celix_version_createVersionFromString("1"); EXPECT_TRUE(version != nullptr); - EXPECT_EQ(1, version->major); - + expectVersion(version, 1, 0, 0); version_destroy(version); - free(str); - str = celix_utils_strdup("a"); - EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, version_createVersionFromString(str, &version)); - - free(str); - str = celix_utils_strdup("1.a"); - EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, version_createVersionFromString(str, &version)); - - free(str); - str = celix_utils_strdup("1.1.a"); - EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, version_createVersionFromString(str, &version)); - - free(str); - str = celix_utils_strdup("-1"); - EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, version_createVersionFromString(str, &version)); + EXPECT_EQ(nullptr, celix_version_createVersionFromString("a")); + EXPECT_EQ(nullptr, celix_version_createVersionFromString("1.a")); + EXPECT_EQ(nullptr, celix_version_createVersionFromString("1.1.a")); + EXPECT_EQ(nullptr, celix_version_createVersionFromString("-1")); + EXPECT_EQ(nullptr, celix_version_createVersionFromString("1.2.3.abc|xyz")); - free(str); - str = celix_utils_strdup("1.2"); - version = nullptr; - EXPECT_EQ(CELIX_SUCCESS, version_createVersionFromString(str, &version)); + version = celix_version_createVersionFromString("1.2"); EXPECT_TRUE(version != nullptr); - EXPECT_EQ(1, version->major); - EXPECT_EQ(2, version->minor); - + expectVersion(version, 1, 2, 0); version_destroy(version); - free(str); - str = celix_utils_strdup("1.2.3"); - version = nullptr; - status = version_createVersionFromString(str, &version); - EXPECT_EQ(CELIX_SUCCESS, status); + version = celix_version_createVersionFromString("1.2.3"); EXPECT_TRUE(version != nullptr); - EXPECT_EQ(1, version->major); - EXPECT_EQ(2, version->minor); - EXPECT_EQ(3, version->micro); - + expectVersion(version, 1, 2, 3); version_destroy(version); - free(str); - str = celix_utils_strdup("1.2.3.abc"); - version = nullptr; - status = version_createVersionFromString(str, &version); - EXPECT_EQ(CELIX_SUCCESS, status); - EXPECT_TRUE(version != nullptr); - EXPECT_EQ(1, version->major); - EXPECT_EQ(2, version->minor); - EXPECT_EQ(3, version->micro); - EXPECT_STREQ("abc", version->qualifier); - version_destroy(version); - free(str); - str = celix_utils_strdup("1.2.3.abc_xyz"); - version = nullptr; - status = version_createVersionFromString(str, &version); - EXPECT_EQ(CELIX_SUCCESS, status); + version = celix_version_createVersionFromString("1.2.3.abc"); EXPECT_TRUE(version != nullptr); - EXPECT_EQ(1, version->major); - EXPECT_EQ(2, version->minor); - EXPECT_EQ(3, version->micro); - EXPECT_STREQ("abc_xyz", version->qualifier); - + expectVersion(version, 1, 2, 3, "abc"); version_destroy(version); - free(str); - str = celix_utils_strdup("1.2.3.abc-xyz"); - version = nullptr; - status = version_createVersionFromString(str, &version); - EXPECT_EQ(CELIX_SUCCESS, status); - EXPECT_TRUE(version != nullptr); - EXPECT_EQ(1, version->major); - EXPECT_EQ(2, version->minor); - EXPECT_EQ(3, version->micro); - EXPECT_STREQ("abc-xyz", version->qualifier); + version = celix_version_createVersionFromString("1.2.3.abc_xyz"); + EXPECT_TRUE(version != nullptr); + expectVersion(version, 1, 2, 3, "abc_xyz"); version_destroy(version); - free(str); - str = celix_utils_strdup("1.2.3.abc|xyz"); - status = version_createVersionFromString(str, &version); - EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, status); - free(str); -} - -TEST_F(VersionTestSuite, createEmptyVersion) { - celix_version_t* version = nullptr; - celix_status_t status = CELIX_SUCCESS; - status = version_createEmptyVersion(&version); - EXPECT_EQ(CELIX_SUCCESS, status); + version = celix_version_createVersionFromString("1.2.3.abc-xyz"); EXPECT_TRUE(version != nullptr); - EXPECT_EQ(0, version->major); - EXPECT_EQ(0, version->minor); - EXPECT_EQ(0, version->micro); - EXPECT_STREQ("", version->qualifier); - + expectVersion(version, 1, 2, 3, "abc-xyz"); version_destroy(version); -} -TEST_F(VersionTestSuite, getters) { - celix_version_t* version = nullptr; - celix_status_t status = CELIX_SUCCESS; - char * str; - int major, minor, micro; - const char *qualifier; - str = celix_utils_strdup("abc"); - status = version_createVersion(1, 2, 3, str, &version); - EXPECT_EQ(CELIX_SUCCESS, status); + //Testing deprecated api + version = nullptr; + EXPECT_EQ(CELIX_SUCCESS, version_createVersionFromString("1", &version)); EXPECT_TRUE(version != nullptr); + expectVersion(version, 1, 0, 0); + version_destroy(version); - version_getMajor(version, &major); - EXPECT_EQ(1, major); - - version_getMinor(version, &minor); - EXPECT_EQ(2, minor); - - version_getMicro(version, µ); - EXPECT_EQ(3, micro); + EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, version_createVersionFromString("a", &version)); +} - version_getQualifier(version, &qualifier); - EXPECT_STREQ("abc", qualifier); +TEST_F(VersionTestSuite, CreateEmptyVersionTest) { + auto* version = celix_version_createEmptyVersion(); + EXPECT_TRUE(version != nullptr); + expectVersion(version, 0, 0, 0, ""); + version_destroy(version); + //Testing deprecated api + version = nullptr; + EXPECT_EQ(CELIX_SUCCESS, version_createEmptyVersion(&version)); + EXPECT_TRUE(version != nullptr); + expectVersion(version, 0, 0, 0, ""); version_destroy(version); - free(str); } -TEST_F(VersionTestSuite, compare) { +TEST_F(VersionTestSuite, CompareTest) { + //TODO update to updated API + celix_version_t* version = nullptr; celix_version_t* compare = nullptr; celix_status_t status = CELIX_SUCCESS; @@ -261,7 +187,7 @@ TEST_F(VersionTestSuite, compare) { EXPECT_EQ(CELIX_SUCCESS, status); EXPECT_TRUE(result < 0); - // Compare againts a lower version + // Compare against a lower version free(str); str = celix_utils_strdup("abc"); version_destroy(compare); @@ -278,7 +204,7 @@ TEST_F(VersionTestSuite, compare) { free(str); } -TEST_F(VersionTestSuite, celix_version_compareToMajorMinor) { +TEST_F(VersionTestSuite, CompareToMajorMinorTest) { auto* version1 = celix_version_create(2, 2, 0, nullptr); auto* version2 = celix_version_create(2, 2, 4, "qualifier"); @@ -300,7 +226,9 @@ TEST_F(VersionTestSuite, celix_version_compareToMajorMinor) { celix_version_destroy(version2); } -TEST_F(VersionTestSuite, toString) { +TEST_F(VersionTestSuite, ToStringTest) { + //TODO update to updated API + celix_version_t* version = nullptr; celix_status_t status = CELIX_SUCCESS; char * str; @@ -333,7 +261,9 @@ TEST_F(VersionTestSuite, toString) { free(str); } -TEST_F(VersionTestSuite,semanticCompatibility) { +TEST_F(VersionTestSuite, SemanticCompatibilityTest) { + //TODO update to updated API + celix_version_t* provider = nullptr; celix_version_t* compatible_user = nullptr; celix_version_t* incompatible_user_by_major = nullptr; @@ -367,7 +297,7 @@ TEST_F(VersionTestSuite,semanticCompatibility) { version_destroy(incompatible_user_by_minor); } -TEST_F(VersionTestSuite, compareEmptyAndNullQualifier) { +TEST_F(VersionTestSuite, CompareEmptyAndNullQualifierTest) { //nullptr or "" qualifier should be the same auto* v1 = celix_version_create(0, 0, 0, nullptr); auto* v2 = celix_version_create(0, 0, 0, ""); @@ -379,7 +309,7 @@ TEST_F(VersionTestSuite, compareEmptyAndNullQualifier) { celix_version_destroy(v2); } -TEST_F(VersionTestSuite, fillString) { +TEST_F(VersionTestSuite, FillStringTest) { // Create a version object auto* version = celix_version_create(1, 2, 3, "alpha"); diff --git a/libs/framework/include/celix/Exception.h b/libs/utils/include/celix/IOException.h similarity index 64% copy from libs/framework/include/celix/Exception.h copy to libs/utils/include/celix/IOException.h index 1675567d..1715801e 100644 --- a/libs/framework/include/celix/Exception.h +++ b/libs/utils/include/celix/IOException.h @@ -19,20 +19,27 @@ #pragma once #include <exception> +#if __cplusplus >= 201703L //C++17 or higher +#include <string_view> +#endif namespace celix { /** - * @brief Celix runtime Exception + * @brief Celix runtime IO Exception */ - class Exception : public std::exception { + class IOException : public std::exception { public: - explicit Exception(std::string msg) : w{std::move(msg)} {} +#if __cplusplus >= 201703L //C++17 or higher + explicit IOException(std::string_view msg) : w{msg} {} +#else + explicit IOException(std::string msg) : w{std::move(msg)} {} +#endif - Exception(const Exception&) = default; - Exception(Exception&&) = default; - Exception& operator=(const Exception&) = default; - Exception& operator=(Exception&&) = default; + IOException(const IOException&) = default; + IOException(IOException&&) = default; + IOException& operator=(const IOException&) = default; + IOException& operator=(IOException&&) = default; [[nodiscard]] const char* what() const noexcept override { return w.c_str(); diff --git a/libs/utils/include/celix/Properties.h b/libs/utils/include/celix/Properties.h index 45bb0c8b..5be5a762 100644 --- a/libs/utils/include/celix/Properties.h +++ b/libs/utils/include/celix/Properties.h @@ -27,6 +27,7 @@ #include "celix_properties.h" #include "celix_utils.h" #include "celix/Version.h" +#include "celix/IOException.h" namespace celix { @@ -75,12 +76,12 @@ namespace celix { first = {}; second = {}; } else { - first = iter.entry.key; + first = iter.key; second = iter.entry.value; } } - celix_properties_iterator_t iter{.index = -1, .entry = {}, ._data = {}}; + celix_properties_iterator_t iter{.index = 0, .key = nullptr, .entry = {}, ._data = {}}; }; @@ -190,11 +191,19 @@ namespace celix { #endif /** - * @brief Wraps C properties, but does not take ownership -> dtor will not destroy properties + * @brief Wrap C properties and returns it as const in a shared_ptr, + * but does not take ownership -> dtor will not destroy C properties. */ static std::shared_ptr<const Properties> wrap(const celix_properties_t* wrapProps) { auto* cp = const_cast<celix_properties_t*>(wrapProps); - return std::shared_ptr<const Properties>{new Properties{cp}}; + return std::shared_ptr<const Properties>{new Properties{cp, false}}; + } + + /** + * @brief Wrap C properties and take ownership -> dtor will destroy C properties. + */ + static Properties own(celix_properties_t* wrapProps) { + return Properties{wrapProps, true}; } /** @@ -598,15 +607,43 @@ namespace celix { #endif - //TODO test - void store(const std::string& file, const std::string& header = {}) const { - celix_properties_store(cProps.get(), file.c_str(), header.empty() ? nullptr : header.c_str()); + + /** + * @brief Store the property set to the given file path. + * + * This function writes the properties in the given set to the specified file path in a format suitable + * for loading with the load() function. + * If a non-empty header string is provided, it will be written as a comment at the beginning of the file. + * + * @param[in] file The file to store the properties to. + * @param[in] header An optional header string to include as a comment at the beginning of the file. + * @throws celix::IOException If an error occurs while writing to the file. + */ +#if __cplusplus >= 201703L //C++17 or higher + void store(std::string_view path, std::string_view header = {}) const { + storeTo(path.data(), header.empty() ? nullptr : header.data()); + } +#else + void store(const std::string& path, const std::string& header = {}) const { + storeTo(path.data(), header.empty() ? nullptr : header.data()); } +#endif - //TODO laod + /** + * @brief Loads properties from the file at the given path. + * @param[in] path The path to the file containing the properties. + * @return A new Properties object containing the properties from the file. + * @throws celix::IOException If the file cannot be opened or read. + */ +#if __cplusplus >= 201703L //C++17 or higher + static celix::Properties load(std::string_view path) { return loadFrom(path.data()); } +#else + static celix::Properties load(const std::string& path) { return loadFrom(path.data()); } +#endif private: - explicit Properties(celix_properties_t* props) : cProps{props, [](celix_properties_t*) { /*nop*/ }} {} + Properties(celix_properties_t* props, bool takeOwnership) : + cProps{props, [ownership = takeOwnership](celix_properties_t* p){ if (ownership) { celix_properties_destroy(p); }}} {} static celix::Properties::ValueType getAndConvertType( const std::shared_ptr<celix_properties_t>& cProperties, @@ -628,6 +665,21 @@ namespace celix { } } + static celix::Properties loadFrom(const char* path) { + auto* cProps = celix_properties_load(path); + if (cProps) { + return celix::Properties::own(cProps); + } + throw celix::IOException{"Cannot load celix::Properties from path " + std::string{path}}; + } + + void storeTo(const char* path, const char* header) const { + auto status = celix_properties_store(cProps.get(), path, header); + if (status != CELIX_SUCCESS) { + throw celix::IOException{"Cannot store celix::Properties to " + std::string{path}}; + } + } + std::shared_ptr<celix_properties_t> cProps; }; diff --git a/libs/utils/include/celix/Version.h b/libs/utils/include/celix/Version.h index ef50be0f..c8d2943d 100644 --- a/libs/utils/include/celix/Version.h +++ b/libs/utils/include/celix/Version.h @@ -26,10 +26,7 @@ namespace celix { - //TODO CxxVersionTestSuite //TODO doxygen - //TODO test in unordered map and set - //TODO test in map and set class Version { public: Version() : @@ -37,7 +34,7 @@ namespace celix { qualifier{celix_version_getQualifier(cVersion.get())} {} #if __cplusplus >= 201703L //C++17 or higher - explicit Version(int major, int minor, int micro, std::string_view qualifier = {}) : + Version(int major, int minor, int micro, std::string_view qualifier = {}) : cVersion{createVersion(celix_version_create(major, minor, micro, qualifier.empty() ? "" : qualifier.data()))}, qualifier{celix_version_getQualifier(cVersion.get())} {} #else @@ -110,7 +107,9 @@ namespace celix { /** * @brief Create a wrap around a C Celix version without taking ownership. */ - explicit Version(celix_version_t* v) : cVersion{v, [](celix_version_t *){/*nop*/}} {} + explicit Version(celix_version_t* v) : + cVersion{v, [](celix_version_t *){/*nop*/}}, + qualifier{celix_version_getQualifier(cVersion.get())} {} std::shared_ptr<celix_version_t> cVersion; std::string qualifier; //cached qualifier of the const char* from celix_version_getQualifier diff --git a/libs/utils/include/celix_long_hash_map.h b/libs/utils/include/celix_long_hash_map.h index c0c36968..aacfef58 100644 --- a/libs/utils/include/celix_long_hash_map.h +++ b/libs/utils/include/celix_long_hash_map.h @@ -69,6 +69,9 @@ typedef struct celix_long_hash_map_create_options { * only the simpleRemovedCallback will be used. * * Default is NULL. + * + * @param[in] removedValue The value that was removed from the hash map. This value is no longer used by the + * hash map and can be freed. */ void (*simpleRemovedCallback)(void* value) CELIX_OPTS_INIT; @@ -89,6 +92,11 @@ typedef struct celix_long_hash_map_create_options { * only the simpleRemovedCallback will be used. * * Default is NULL. + * + * @param[in] data The void pointer to the data that was provided when the callback was set as removedCallbackData. + * @param[in] removedKey The key of the value that was removed from the hash map. + * @param[in] removedValue The value that was removed from the hash map. This value is no longer used by the + * hash map and can be freed. */ void (*removedCallback)(void* data, long removedKey, celix_hash_map_value_t removedValue) CELIX_OPTS_INIT; diff --git a/libs/utils/include/celix_properties.h b/libs/utils/include/celix_properties.h index f20a2fe8..71bc0f53 100644 --- a/libs/utils/include/celix_properties.h +++ b/libs/utils/include/celix_properties.h @@ -70,7 +70,6 @@ typedef enum celix_properties_value_type { * @brief A structure representing a single entry in a property set. */ typedef struct celix_properties_entry { - const char* key; /**< The key of the entry*/ const char* value; /**< The string value or string representation of a non-string typed value.*/ celix_properties_value_type_e valueType; /**< The type of the value of the entry */ @@ -94,10 +93,15 @@ typedef struct celix_properties_iterator { /** * @brief The index of the current iterator. */ - int index; + size_t index; /** - * @brief The current entry. + * @brief Te current key. + */ + const char* key; + + /** + * @brief The current value entry. */ celix_properties_entry_t entry; @@ -152,6 +156,8 @@ celix_properties_t* celix_properties_loadFromString(const char *input); /** * @brief Store properties to a file. * + * @note Properties values are always stored as string values, regardless of their actual underlining types. + * * @param[in] properties The property set to store. * @param[in] file The name of the file to store the properties to. * @param[in] header An optional header to write to the file before the properties. @@ -200,6 +206,8 @@ void celix_properties_set(celix_properties_t* properties, const char* key, const /** * @brief Set the value of a property without copying the key and value strings. * + * @note If the setWithoutCopy replaced an exising value, the key will be freed by callee. + * * @param[in] properties The property set to modify. * @param[in] key The key of the property to set. This string will be used directly, so it must not be freed or modified * after calling this function. diff --git a/libs/utils/include/celix_string_hash_map.h b/libs/utils/include/celix_string_hash_map.h index 323755cb..7dfe661f 100644 --- a/libs/utils/include/celix_string_hash_map.h +++ b/libs/utils/include/celix_string_hash_map.h @@ -68,11 +68,14 @@ typedef struct celix_string_hash_map_create_options { * only the simpledRemoveCallback will be used. * * Default is NULL. + * + * @param[in] removedValue The value that was removed from the hash map. This value is no longer used by the + * hash map and can be freed. */ - void (*simpleRemovedCallback)(void* value) CELIX_OPTS_INIT; + void (*simpleRemovedCallback)(void* removedValue) CELIX_OPTS_INIT; /** - * Optional callback data, which will be provided to the removedCallback callback. + * Optional callback data, which will be provided to the removedCallback and removedKeyCallback callback. * * Default is NULL. */ @@ -88,14 +91,37 @@ typedef struct celix_string_hash_map_create_options { * only the simpledRemoveCallback will be used. * * Default is NULL. + * + * @param[in] data The void pointer to the data that was provided when the callback was set as removedCallbackData. + * @param[in] removedKey The key of the value that was removed from the hash map. + * Note that the removedKey can still be in use if the a value entry for the same key is + * replaced, so this callback should not free the removedKey. + * @param[in] removedValue The value that was removed from the hash map. This value is no longer used by the + * hash map and can be freed. */ void (*removedCallback)(void* data, const char* removedKey, celix_hash_map_value_t removedValue) CELIX_OPTS_INIT; + /** + * @brief A removed key callback, which if provided will be called if a key is no longer used in the hash map. + * + * @param[in] data The void pointer to the data that was provided when the callback was set as removedCallbackData. + * @param[in] key The key that is no longer used in the hash map. if `storeKeysWeakly` was configured as true, + * the key can be freed. + */ + void (*removedKeyCallback)(void* data, char* key) CELIX_OPTS_INIT; + /** * @brief If set to true, the string hash map will not make of copy of the keys and assumes * that the keys are in scope/memory for the complete lifecycle of the string hash map. * - * Note that this changes the default behaviour of the celix_stringHashMap_put* functions. + * When keys are stored weakly it is the caller responsibility to check the return value of + * celix_stringHashMap_put* function calls. + * If a celix_stringHashMap_put* function call returns true, the key is used in the hash map and the key + * should never be freed or freed in a configured removedKeyCallback. + * If a celix_stringHashMap_put* function call returns false, a value is replaced and the already existing + * key is reused. If the needed the caller should free the provided key. + * + * @note This changes the default behaviour of the celix_stringHashMap_put* functions. * * Default is false. */ @@ -137,6 +163,7 @@ typedef struct celix_string_hash_map_create_options { .simpleRemovedCallback = NULL, \ .removedCallbackData = NULL, \ .removedCallback = NULL, \ + .removedKeyCallback = NULL, \ .storeKeysWeakly = false, \ .initialCapacity = 0, \ .loadFactor = 0 \ diff --git a/libs/utils/src/celix_hash_map.c b/libs/utils/src/celix_hash_map.c index 42e37ec6..995b0e6b 100644 --- a/libs/utils/src/celix_hash_map.c +++ b/libs/utils/src/celix_hash_map.c @@ -73,8 +73,9 @@ typedef struct celix_hash_map { bool (*equalsKeyFunction)(const celix_hash_map_key_t* key1, const celix_hash_map_key_t* key2); void (*simpleRemovedCallback)(void* value); void* removedCallbackData; - void (*removedStringKeyCallback)(void* data, const char* removedKey, celix_hash_map_value_t removedValue); - void (*removedLongKeyCallback)(void* data, long removedKey, celix_hash_map_value_t removedValue); + void (*removedStringEntryCallback)(void* data, const char* removedKey, celix_hash_map_value_t removedValue); + void (*removedStringKeyCallback)(void* data, char* key); + void (*removedLongEntryCallback)(void* data, long removedKey, celix_hash_map_value_t removedValue); bool storeKeysWeakly; } celix_hash_map_t; @@ -227,18 +228,24 @@ static void celix_hashMap_resize(celix_hash_map_t* map, size_t newCapacity) { static void celix_hashMap_callRemovedCallback(celix_hash_map_t* map, celix_hash_map_entry_t* removedEntry) { if (map->simpleRemovedCallback) { map->simpleRemovedCallback(removedEntry->value.ptrValue); - } else if (map->removedLongKeyCallback) { - map->removedLongKeyCallback(map->removedCallbackData, removedEntry->key.longKey, removedEntry->value); - } else if (map->removedStringKeyCallback) { - map->removedStringKeyCallback(map->removedCallbackData, removedEntry->key.strKey, removedEntry->value); + } else if (map->removedLongEntryCallback) { + map->removedLongEntryCallback(map->removedCallbackData, removedEntry->key.longKey, removedEntry->value); + } else if (map->removedStringEntryCallback) { + map->removedStringEntryCallback(map->removedCallbackData, removedEntry->key.strKey, removedEntry->value); + } +} + +static void celix_hashMap_destroyRemovedKey(celix_hash_map_t* map, char* removedKey) { + if (map->removedStringKeyCallback) { + map->removedStringKeyCallback(map->removedCallbackData, removedKey); + } + if (!map->storeKeysWeakly) { + free(removedKey); } } static void celix_hashMap_destroyRemovedEntry(celix_hash_map_t* map, celix_hash_map_entry_t* removedEntry) { celix_hashMap_callRemovedCallback(map, removedEntry); - if (map->keyType == CELIX_HASH_MAP_STRING_KEY && !map->storeKeysWeakly) { - free((char*)removedEntry->key.strKey); - } free(removedEntry); } @@ -344,7 +351,14 @@ static bool celix_hashMap_remove(celix_hash_map_t* map, const char* strKey, long visit = visit->next; } if (removedEntry != NULL) { + char* removedKey = NULL; + if (map->keyType == CELIX_HASH_MAP_STRING_KEY) { + removedKey = (char*)removedEntry->key.strKey; + } celix_hashMap_destroyRemovedEntry(map, removedEntry); + if (removedKey) { + celix_hashMap_destroyRemovedKey(map, removedKey); + } return true; } return false; @@ -367,7 +381,8 @@ static void celix_hashMap_init( map->equalsKeyFunction = equalsKeyFn; map->simpleRemovedCallback = NULL; map->removedCallbackData = NULL; - map->removedLongKeyCallback = NULL; + map->removedLongEntryCallback = NULL; + map->removedStringEntryCallback = NULL; map->removedStringKeyCallback = NULL; map->storeKeysWeakly = false; } @@ -378,7 +393,15 @@ static void celix_hashMap_clear(celix_hash_map_t* map) { while (entry != NULL) { celix_hash_map_entry_t* removedEntry = entry; entry = entry->next; + \ + char* removedKey = NULL; + if (map->keyType == CELIX_HASH_MAP_STRING_KEY) { + removedKey = (char*)removedEntry->key.strKey; + } celix_hashMap_destroyRemovedEntry(map, removedEntry); + if (removedKey) { + celix_hashMap_destroyRemovedKey(map, removedKey); + } } map->buckets[i] = NULL; } @@ -427,7 +450,8 @@ celix_string_hash_map_t* celix_stringHashMap_createWithOptions(const celix_strin celix_hashMap_init(&map->genericMap, CELIX_HASH_MAP_STRING_KEY, cap, fac, celix_stringHashMap_hash, celix_stringHashMap_equals); map->genericMap.simpleRemovedCallback = opts->simpleRemovedCallback; map->genericMap.removedCallbackData = opts->removedCallbackData; - map->genericMap.removedStringKeyCallback = opts->removedCallback; + map->genericMap.removedStringEntryCallback = opts->removedCallback; + map->genericMap.removedStringKeyCallback = opts->removedKeyCallback; map->genericMap.storeKeysWeakly = opts->storeKeysWeakly; return map; } @@ -444,7 +468,7 @@ celix_long_hash_map_t* celix_longHashMap_createWithOptions(const celix_long_hash celix_hashMap_init(&map->genericMap, CELIX_HASH_MAP_LONG_KEY, cap, fac, celix_longHashMap_hash, celix_longHashMap_equals); map->genericMap.simpleRemovedCallback = opts->simpleRemovedCallback; map->genericMap.removedCallbackData = opts->removedCallbackData; - map->genericMap.removedLongKeyCallback = opts->removedCallback; + map->genericMap.removedLongEntryCallback = opts->removedCallback; map->genericMap.storeKeysWeakly = false; return map; } diff --git a/libs/utils/src/properties.c b/libs/utils/src/properties.c index 422a2bb7..dbbc2048 100644 --- a/libs/utils/src/properties.c +++ b/libs/utils/src/properties.c @@ -160,14 +160,12 @@ static char* celix_properties_createString(celix_properties_t* properties, const static celix_status_t celix_properties_fillEntry( celix_properties_t *properties, celix_properties_entry_t* entry, - const char *key, const char *strValue, const long* longValue, const double* doubleValue, const bool* boolValue, celix_version_t* versionValue) { char convertedValueBuffer[32]; - entry->key = celix_properties_createString(properties, key); if (strValue != NULL) { entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_STRING; entry->value = celix_properties_createString(properties, strValue); @@ -209,7 +207,7 @@ static celix_status_t celix_properties_fillEntry( entry->value = celix_version_toString(versionValue); } } - if (entry->key == NULL || entry->value == NULL) { + if (entry->value == NULL) { return CELIX_ENOMEM; } return CELIX_SUCCESS; @@ -233,13 +231,11 @@ static celix_properties_entry_t* celix_properties_allocEntry(celix_properties_t* * provided key and value strings. */ static celix_properties_entry_t* celix_properties_createEntryWithNoCopy(celix_properties_t *properties, - char *key, char *strValue) { celix_properties_entry_t* entry = celix_properties_allocEntry(properties); if (entry == NULL) { return NULL; } - entry->key = key; entry->value = strValue; entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_STRING; entry->typed.strValue = strValue; @@ -264,7 +260,7 @@ static celix_properties_entry_t* celix_properties_createEntry( return NULL; } - celix_status_t status = celix_properties_fillEntry(properties, entry, key, strValue, longValue, doubleValue, + celix_status_t status = celix_properties_fillEntry(properties, entry, strValue, longValue, doubleValue, boolValue, versionValue); if (status != CELIX_SUCCESS) { free(entry); @@ -290,8 +286,13 @@ static void celix_properties_createAndSetEntry( } celix_properties_entry_t* entry = celix_properties_createEntry(properties, key, strValue, longValue, doubleValue, boolValue, versionValue); + const char* mapKey = key; + if (!celix_stringHashMap_hasKey(properties->map, key)) { + //new entry, needs new allocated key; + mapKey = celix_properties_createString(properties, key); + } if (entry != NULL) { - celix_stringHashMap_put(properties->map, entry->key, entry); + celix_stringHashMap_put(properties->map, mapKey, entry); } } @@ -308,10 +309,14 @@ static void celix_properties_freeString(celix_properties_t* properties, char* st } } +static void celix_properties_removeKeyCallback(void* handle, char* key) { + celix_properties_t* properties = handle; + celix_properties_freeString(properties, key); +} + static void celix_properties_removeEntryCallback(void* handle, const char* key __attribute__((unused)), celix_hash_map_value_t val) { celix_properties_t* properties = handle; celix_properties_entry_t* entry = val.ptrValue; - celix_properties_freeString(properties, (char*)entry->key); celix_properties_freeString(properties, (char*)entry->value); if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_VERSION) { celix_version_destroy(entry->typed.versionValue); @@ -332,6 +337,7 @@ celix_properties_t* celix_properties_create(void) { opts.initialCapacity = (unsigned int)ceil(CELIX_SHORT_PROPERTIES_OPTIMIZATION_ENTRIES_SIZE / 0.75); opts.removedCallbackData = props; opts.removedCallback = celix_properties_removeEntryCallback; + opts.removedKeyCallback = celix_properties_removeKeyCallback; props->map = celix_stringHashMap_createWithOptions(&opts); props->currentStringBufferIndex = 0; props->currentEntriesBufferIndex = 0; @@ -585,16 +591,16 @@ celix_properties_t* celix_properties_copy(const celix_properties_t *properties) CELIX_PROPERTIES_ITERATE(properties, iter) { if (iter.entry.valueType == CELIX_PROPERTIES_VALUE_TYPE_STRING) { - celix_properties_set(copy, iter.entry.key, iter.entry.value); + celix_properties_set(copy, iter.key, iter.entry.value); } else if (iter.entry.valueType == CELIX_PROPERTIES_VALUE_TYPE_LONG) { - celix_properties_setLong(copy, iter.entry.key, iter.entry.typed.longValue); + celix_properties_setLong(copy, iter.key, iter.entry.typed.longValue); } else if (iter.entry.valueType == CELIX_PROPERTIES_VALUE_TYPE_DOUBLE) { - celix_properties_setDouble(copy, iter.entry.key, iter.entry.typed.doubleValue); + celix_properties_setDouble(copy, iter.key, iter.entry.typed.doubleValue); } else if (iter.entry.valueType == CELIX_PROPERTIES_VALUE_TYPE_BOOL) { - celix_properties_setBool(copy, iter.entry.key, iter.entry.typed.boolValue); + celix_properties_setBool(copy, iter.key, iter.entry.typed.boolValue); } else /*version*/ { assert(iter.entry.valueType == CELIX_PROPERTIES_VALUE_TYPE_VERSION); - celix_properties_setVersion(copy, iter.entry.key, iter.entry.typed.versionValue); + celix_properties_setVersion(copy, iter.key, iter.entry.typed.versionValue); } } return copy; @@ -629,9 +635,12 @@ void celix_properties_set(celix_properties_t *properties, const char *key, const void celix_properties_setWithoutCopy(celix_properties_t *properties, char *key, char *value) { if (properties != NULL && key != NULL && value != NULL) { - celix_properties_entry_t* entry = celix_properties_createEntryWithNoCopy(properties, key, value); + celix_properties_entry_t* entry = celix_properties_createEntryWithNoCopy(properties, value); if (entry != NULL) { - celix_stringHashMap_put(properties->map, entry->key, entry); + bool replaced = celix_stringHashMap_put(properties->map, key, entry); + if (replaced) { + free(key); + } } } } @@ -755,7 +764,8 @@ celix_properties_iterator_t celix_propertiesIterator_construct(const celix_prope internalIter.props = properties; celix_properties_iterator_t iter; - iter.index = -1; + iter.index = 0; + iter.key = NULL; memset(&iter.entry, 0, sizeof(iter.entry)); CELIX_BUILD_ASSERT(sizeof(celix_properties_iterator_internal_t) <= sizeof(iter._data)); @@ -767,7 +777,6 @@ celix_properties_iterator_t celix_propertiesIterator_construct(const celix_prope bool celix_propertiesIterator_hasNext(celix_properties_iterator_t *iter) { celix_properties_iterator_internal_t internalIter; memcpy(&internalIter, iter->_data, sizeof(internalIter)); - //celix_stringHashMapIterator_next(&internalIter.mapIter); return !celix_stringHashMapIterator_isEnd(&internalIter.mapIter); } @@ -780,8 +789,10 @@ const char* celix_propertiesIterator_nextKey(celix_properties_iterator_t *iter) iter->index = (int)internalIter.mapIter.index; celix_properties_entry_t* entry = internalIter.mapIter.value.ptrValue; if (entry != NULL) { + iter->key = internalIter.mapIter.key; memcpy(&iter->entry, entry, sizeof(iter->entry)); } else { + iter->key = NULL; memset(&iter->entry, 0, sizeof(iter->entry)); } celix_stringHashMapIterator_next(&internalIter.mapIter); @@ -799,8 +810,10 @@ celix_properties_iterator_t celix_properties_begin(const celix_properties_t* pro celix_properties_iterator_t iter; iter.index = 0; if (celix_stringHashMapIterator_isEnd(&internalIter.mapIter)) { + iter.key = NULL; memset(&iter.entry, 0, sizeof(iter.entry)); } else { + iter.key = internalIter.mapIter.key; memcpy(&iter.entry, internalIter.mapIter.value.ptrValue, sizeof(iter.entry)); } @@ -816,7 +829,7 @@ celix_properties_iterator_t celix_properties_end(const celix_properties_t* prope celix_properties_iterator_t iter; memset(&iter, 0, sizeof(iter)); - iter.index = (int)internalIter.mapIter.index; //TODO make inter.index size_t + iter.index = internalIter.mapIter.index; memcpy(iter._data, &internalIter, sizeof(internalIter)); return iter; } @@ -828,8 +841,10 @@ void celix_propertiesIterator_next(celix_properties_iterator_t *iter) { memcpy(iter->_data, &internalIter, sizeof(internalIter)); iter->index = internalIter.mapIter.index; if (celix_stringHashMapIterator_isEnd(&internalIter.mapIter)) { + iter->key = NULL; memset(&iter->entry, 0, sizeof(iter->entry)); } else { + iter->key = internalIter.mapIter.key; memcpy(&iter->entry, internalIter.mapIter.value.ptrValue, sizeof(iter->entry)); } } diff --git a/libs/utils/src/version.c b/libs/utils/src/version.c index 72a37942..b44b123f 100644 --- a/libs/utils/src/version.c +++ b/libs/utils/src/version.c @@ -107,7 +107,7 @@ celix_version_t* celix_version_create(int major, int minor, int micro, const cha } if (qualifier == NULL) { - qualifier = ""; + qualifier = CELIX_VERSION_EMPTY_QUALIFIER; } size_t qualifierLen = strlen(qualifier); for (int i = 0; i < qualifierLen; i++) {
