This is an automated email from the ASF dual-hosted git repository. pnoltes pushed a commit to branch feature/674-improve-properties in repository https://gitbox.apache.org/repos/asf/celix.git
commit ebfbb7ea0e0ef7daf29078adae814148c878fe7c Author: Pepijn Noltes <[email protected]> AuthorDate: Tue Jan 2 18:01:42 2024 +0100 Refactor properties get version non-owning func names --- documents/services.md | 2 +- libs/framework/src/bundle_context.c | 2 +- libs/framework/src/dm_component_impl.c | 4 ++-- libs/utils/gtest/src/PropertiesTestSuite.cc | 20 +++++++++---------- libs/utils/include/celix/Properties.h | 3 +-- libs/utils/include/celix_properties.h | 30 ++++++++++++++++------------- libs/utils/src/properties.c | 7 ++++--- 7 files changed, 36 insertions(+), 32 deletions(-) diff --git a/documents/services.md b/documents/services.md index 47189591..ed79d21b 100644 --- a/documents/services.md +++ b/documents/services.md @@ -58,7 +58,7 @@ The following C functions can be used to create and manipulate properties: - `celix_properties_getAsLong` - Get a long property value or string value parsed as long. - `celix_properties_getAsDouble` - Get a double property value or string value parsed as double. - `celix_properties_getAsBool` - Get a bool property value or string value parsed as bool. -- `celix_properties_getVersion` - Get a pointer to the version property if the property value is a version or was +- `celix_properties_getAsVersion` - Get a pointer to the version property if the property value is a version or was parsable as a version. Note there is also a `celix_properties_getAsVersion` function which return a new version object, but this is less efficient and requires a memory allocation. diff --git a/libs/framework/src/bundle_context.c b/libs/framework/src/bundle_context.c index 7985fcf3..899c2a18 100644 --- a/libs/framework/src/bundle_context.c +++ b/libs/framework/src/bundle_context.c @@ -406,7 +406,7 @@ static long celix_bundleContext_registerServiceWithOptionsInternal(bundle_contex return -1; } celix_status_t rc = - celix_properties_setVersionWithoutCopy(props, CELIX_FRAMEWORK_SERVICE_VERSION, celix_steal_ptr(version)); + celix_properties_assignVersion(props, CELIX_FRAMEWORK_SERVICE_VERSION, celix_steal_ptr(version)); if (rc != CELIX_SUCCESS) { celix_framework_logTssErrors(ctx->framework->logger, CELIX_LOG_LEVEL_ERROR); fw_log(ctx->framework->logger, CELIX_LOG_LEVEL_ERROR, "Cannot set service version %s", opts->serviceVersion); diff --git a/libs/framework/src/dm_component_impl.c b/libs/framework/src/dm_component_impl.c index d4c3ea60..ba58f591 100644 --- a/libs/framework/src/dm_component_impl.c +++ b/libs/framework/src/dm_component_impl.c @@ -472,8 +472,8 @@ celix_status_t celix_dmComponent_addInterface(celix_dm_component_t* component, celix_properties_destroy(properties); return CELIX_ILLEGAL_ARGUMENT; } - celix_status_t rc = celix_properties_setVersionWithoutCopy( - properties, CELIX_FRAMEWORK_SERVICE_VERSION, celix_steal_ptr(version)); + celix_status_t rc = + celix_properties_assignVersion(properties, CELIX_FRAMEWORK_SERVICE_VERSION, celix_steal_ptr(version)); if (rc != CELIX_SUCCESS) { celix_bundleContext_log( component->context, CELIX_LOG_LEVEL_ERROR, "Cannot add interface with an invalid serviceVersion"); diff --git a/libs/utils/gtest/src/PropertiesTestSuite.cc b/libs/utils/gtest/src/PropertiesTestSuite.cc index 3e395908..4faa8f88 100644 --- a/libs/utils/gtest/src/PropertiesTestSuite.cc +++ b/libs/utils/gtest/src/PropertiesTestSuite.cc @@ -335,8 +335,8 @@ TEST_F(PropertiesTestSuite, GetSetOverwrite) { EXPECT_EQ(2.0, celix_properties_getAsLong(props, "key", -2.0)); EXPECT_EQ(CELIX_SUCCESS, celix_properties_setBool(props, "key", false)); EXPECT_EQ(false, celix_properties_getAsBool(props, "key", true)); - EXPECT_EQ(CELIX_SUCCESS, celix_properties_setVersionWithoutCopy(props, "key", version)); - EXPECT_EQ(version, celix_properties_getVersion(props, "key", nullptr)); + EXPECT_EQ(CELIX_SUCCESS, celix_properties_assignVersion(props, "key", version)); + EXPECT_EQ(version, celix_properties_peekVersion(props, "key", nullptr)); celix_properties_set(props, "key", "last"); celix_properties_destroy(props); @@ -498,7 +498,7 @@ TEST_F(PropertiesTestSuite, GetVersionTest) { // Test getting a version property auto* expected = celix_version_create(1, 2, 3, "test"); celix_properties_setVersion(properties, "key", expected); - const auto* actual = celix_properties_getVersion(properties, "key", nullptr); + const auto* actual = celix_properties_peekVersion(properties, "key", nullptr); EXPECT_EQ(celix_version_getMajor(expected), celix_version_getMajor(actual)); EXPECT_EQ(celix_version_getMinor(expected), celix_version_getMinor(actual)); EXPECT_EQ(celix_version_getMicro(expected), celix_version_getMicro(actual)); @@ -506,17 +506,17 @@ TEST_F(PropertiesTestSuite, GetVersionTest) { // Test getting a non-version property celix_properties_set(properties, "key2", "value"); - actual = celix_properties_getVersion(properties, "key2", emptyVersion); + actual = celix_properties_peekVersion(properties, "key2", emptyVersion); EXPECT_EQ(celix_version_getMajor(actual), 0); EXPECT_EQ(celix_version_getMinor(actual), 0); EXPECT_EQ(celix_version_getMicro(actual), 0); EXPECT_STREQ(celix_version_getQualifier(actual), ""); - EXPECT_EQ(celix_properties_getVersion(properties, "non-existent", nullptr), nullptr); + EXPECT_EQ(celix_properties_peekVersion(properties, "non-existent", nullptr), nullptr); celix_version_destroy(expected); // Test setting without copy - celix_properties_setVersionWithoutCopy(properties, "key3", celix_version_create(3,3,3,"")); - actual = celix_properties_getVersion(properties, "key3", emptyVersion); + celix_properties_assignVersion(properties, "key3", celix_version_create(3, 3, 3, "")); + actual = celix_properties_peekVersion(properties, "key3", emptyVersion); EXPECT_EQ(celix_version_getMajor(actual), 3); EXPECT_EQ(celix_version_getMinor(actual), 3); EXPECT_EQ(celix_version_getMicro(actual), 3); @@ -656,9 +656,9 @@ TEST_F(PropertiesTestSuite, PropertiesEqualsTest) { celix_properties_setBool(prop2, "key3", false); EXPECT_TRUE(celix_properties_equals(prop1, prop2)); - celix_properties_setVersionWithoutCopy(prop1, "key4", celix_version_create(1,2,3, nullptr)); + celix_properties_assignVersion(prop1, "key4", celix_version_create(1, 2, 3, nullptr)); EXPECT_FALSE(celix_properties_equals(prop1, prop2)); - celix_properties_setVersionWithoutCopy(prop2, "key4", celix_version_create(1,2,3, nullptr)); + celix_properties_assignVersion(prop2, "key4", celix_version_create(1, 2, 3, nullptr)); EXPECT_TRUE(celix_properties_equals(prop1, prop2)); celix_properties_setLong(prop1, "key5", 42); @@ -673,7 +673,7 @@ TEST_F(PropertiesTestSuite, PropertiesNullArgumentsTest) { EXPECT_EQ(CELIX_SUCCESS, celix_properties_setDouble(nullptr, "key", 1.0)); EXPECT_EQ(CELIX_SUCCESS, celix_properties_setBool(nullptr, "key", true)); EXPECT_EQ(CELIX_SUCCESS, celix_properties_setVersion(nullptr, "key", nullptr)); - EXPECT_EQ(CELIX_SUCCESS, celix_properties_setVersionWithoutCopy(nullptr, "key", nullptr)); + EXPECT_EQ(CELIX_SUCCESS, celix_properties_assignVersion(nullptr, "key", nullptr)); celix_autoptr(celix_properties_t) copy = celix_properties_copy(nullptr); EXPECT_NE(nullptr, copy); } diff --git a/libs/utils/include/celix/Properties.h b/libs/utils/include/celix/Properties.h index eed6f510..8dd89f2d 100644 --- a/libs/utils/include/celix/Properties.h +++ b/libs/utils/include/celix/Properties.h @@ -340,14 +340,13 @@ namespace celix { * or the value is not a Celix version. */ celix::Version getAsVersion(const std::string& key, celix::Version defaultValue = {}) { - auto* cVersion = celix_properties_getAsVersion(cProps.get(), key.data(), nullptr); + const auto* cVersion = celix_properties_peekVersion(cProps.get(), key.data(), nullptr); if (cVersion) { celix::Version version{ celix_version_getMajor(cVersion), celix_version_getMinor(cVersion), celix_version_getMicro(cVersion), celix_version_getQualifier(cVersion)}; - celix_version_destroy(cVersion); return version; } return defaultValue; diff --git a/libs/utils/include/celix_properties.h b/libs/utils/include/celix_properties.h index 594c0eb7..f47028c8 100644 --- a/libs/utils/include/celix_properties.h +++ b/libs/utils/include/celix_properties.h @@ -342,7 +342,7 @@ CELIX_UTILS_EXPORT celix_status_t celix_properties_setVersion(celix_properties_t const celix_version_t* version); /** - * @brief Set the value of a property as a Celix version. + * @brief Assign the value of a property with the provided Celix version pointer. * * This function will store a reference to the provided celix_version_t object in the property set and takes * ownership of the provided version. @@ -350,33 +350,37 @@ CELIX_UTILS_EXPORT celix_status_t celix_properties_setVersion(celix_properties_t * * @param[in] properties The property set to modify. * @param[in] key The key of the property to set. - * @param[in] version The value to set. The function will store a reference to this object in the property set and + * @param[in] version The value to assign. The function will store a reference to this object in the property set and * takes ownership of the provided version. @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. When an error status is returned, * the version will be destroy with celix_version_destroy by this function. */ -CELIX_UTILS_EXPORT celix_status_t -celix_properties_setVersionWithoutCopy(celix_properties_t* properties, const char* key, celix_version_t* version); +CELIX_UTILS_EXPORT celix_status_t celix_properties_assignVersion(celix_properties_t* properties, + const char* key, + celix_version_t* version); /** - * @brief Get the Celix version value of a property. + * @brief Peek at the Celix version value of a property without copying. * - * This function does not convert a string property value to a Celix version automatically. + * This function provides a non-owning, read-only access to a Celix version contained in the properties. + * It returns a const pointer to the Celix version value associated with the specified key. + * This function does not perform any conversion from a string property value to a Celix version. * * @param[in] properties The property set to search. - * @param[in] key The key of the property to get. + * @param[in] key The key of the property to peek at. * @param[in] defaultValue The value to return if the property is not set or if the value is not a Celix version. - * @return The value of the property if it is a Celix version, or the default value if the property is not set or the - * value is not a Celix version. + * @return A const pointer to the Celix version if it is present and valid, or the provided default value if the + * property is not set or the value is not a valid Celix version. The returned pointer should not be modified or freed. */ -CELIX_UTILS_EXPORT const celix_version_t* -celix_properties_getVersion(const celix_properties_t* properties, const char* key, const celix_version_t* defaultValue); +CELIX_UTILS_EXPORT const celix_version_t* celix_properties_peekVersion(const celix_properties_t* properties, + const char* key, + const celix_version_t* defaultValue); /** - * @brief Get the value of a property as a Celix version. + * @brief Get a value of a property as a copied Celix version. * - * If the property value is a Celix version, a copy of this version will be returned. + * If the property value is a Celix version, a copy of the found version will be returned. * If the property value is a string, this function will attempt to convert it to a new Celix version. * If the property is not set or is not a valid Celix version string, a copy of the provided defaultValue is returned. * diff --git a/libs/utils/src/properties.c b/libs/utils/src/properties.c index 8d3a2dfa..ea4f1a48 100644 --- a/libs/utils/src/properties.c +++ b/libs/utils/src/properties.c @@ -806,7 +806,7 @@ celix_status_t celix_properties_setBool(celix_properties_t* props, const char* k return celix_properties_createAndSetEntry(props, key, &prototype); } -const celix_version_t* celix_properties_getVersion(const celix_properties_t* properties, +const celix_version_t* celix_properties_peekVersion(const celix_properties_t* properties, const char* key, const celix_version_t* defaultValue) { celix_properties_entry_t* entry = celix_properties_getEntry(properties, key); @@ -846,11 +846,12 @@ celix_properties_setVersion(celix_properties_t* props, const char* key, const ce return celix_properties_createAndSetEntry(props, key, &prototype); } -celix_status_t celix_properties_setVersionWithoutCopy(celix_properties_t* props, const char* key, celix_version_t* version) { +celix_status_t +celix_properties_assignVersion(celix_properties_t* properties, const char* key, celix_version_t* version) { celix_properties_entry_t prototype = {0}; prototype.valueType = CELIX_PROPERTIES_VALUE_TYPE_VERSION; prototype.typed.versionValue = version; - return celix_properties_createAndSetEntry(props, key, &prototype); + return celix_properties_createAndSetEntry(properties, key, &prototype); } size_t celix_properties_size(const celix_properties_t* properties) {
