gh-685: Fix tms_tests and improve several error handling issues

bundles/remote_services/topology_manager/tms_tst/CMakeLists.txt

@@ -57,15 +57,17 @@ file(GENERATE
     CONTENT "{
     \"CELIX_AUTO_START_1\":\"$<TARGET_PROPERTY:Celix::rsa_dfi,BUNDLE_FILE>,$<TARGET_PROPERTY:calculator,BUNDLE_FILE>,$<TARGET_PROPERTY:Celix::rsa_topology_manager,BUNDLE_FILE>,$<TARGET_PROPERTY:topology_manager_disc_mock_bundle,BUNDLE_FILE>\",
     \"LOGHELPER_ENABLE_STDOUT_FALLBACK\":true,
-    \"CELIX_FRAMEWORK_CLEAN_CACHE_DIR_ON_CREATE\":true
+    \"CELIX_FRAMEWORK_CLEAN_CACHE_DIR_ON_CREATE\":true,
+    \"CELIX_FRAMEWORK_CACHE_DIR\":\".rstm_cache\"
 }")
 
 file(GENERATE 
     OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/config_import.properties"
     CONTENT "{
     \"CELIX_AUTO_START_1\":\"$<TARGET_PROPERTY:Celix::rsa_dfi,BUNDLE_FILE>,$<TARGET_PROPERTY:calculator,BUNDLE_FILE>,$<TARGET_PROPERTY:Celix::rsa_topology_manager,BUNDLE_FILE>,$<TARGET_PROPERTY:topology_manager_test_bundle,BUNDLE_FILE>\",
     \"LOGHELPER_ENABLE_STDOUT_FALLBACK\":true,
-    \"CELIX_FRAMEWORK_CLEAN_CACHE_DIR_ON_CREATE\":true
+    \"CELIX_FRAMEWORK_CLEAN_CACHE_DIR_ON_CREATE\":true,
+    \"CELIX_FRAMEWORK_CACHE_DIR\":\".rstm_import_cache\"
 }")
 
 configure_file("scope.json" "scope.json")

bundles/remote_services/topology_manager/tms_tst/tms_tests.cpp

@@ -24,6 +24,7 @@
 
 #include "celix_bundle_context.h"
 #include "celix_framework_factory.h"
+#include "celix_version.h"
 
 extern "C" {
 
@@ -172,7 +173,7 @@ extern "C" {
 
     static void setupFmImport() {
         celix_properties_t* config;
-        ASSERT_EQ(CELIX_SUCCESS, celix_properties_load("config.properties", 0, &config));
+        ASSERT_EQ(CELIX_SUCCESS, celix_properties_load("config_import.properties", 0, &config));
 
         framework = celix_frameworkFactory_createFramework(config);
         ASSERT_NE(nullptr, framework);
@@ -484,7 +485,8 @@ extern "C" {
         celix_properties_set(props, CELIX_RSA_ENDPOINT_ID, "eec5404d-51d0-47ef-8d86-c825a8beda42-42");
         celix_properties_set(props, CELIX_RSA_SERVICE_IMPORTED_CONFIGS, TST_CONFIGURATION_TYPE);
         celix_properties_set(props, CELIX_FRAMEWORK_SERVICE_NAME, "org.apache.celix.test.MyBundle");
-        celix_properties_set(props, "service.version", "1.0.0");
+        auto* v = celix_version_create(1, 0, 0, nullptr);
+        celix_properties_assignVersion(props, "service.version", v);
         celix_properties_set(props, "zone", "a_zone");
 
         rc = endpointDescription_create(props, &endpoint);
@@ -536,7 +538,8 @@ extern "C" {
         celix_properties_set(props, CELIX_RSA_ENDPOINT_ID, "eec5404d-51d0-47ef-8d86-c825a8beda42-42");
         celix_properties_set(props, CELIX_RSA_SERVICE_IMPORTED_CONFIGS, TST_CONFIGURATION_TYPE);
         celix_properties_set(props, CELIX_FRAMEWORK_SERVICE_NAME, "org.apache.celix.test.MyBundle");
-        celix_properties_set(props, "service.version", "1.0.0");
+        auto* v = celix_version_create(1, 0, 0, nullptr);
+        celix_properties_assignVersion(props, "service.version", v);
         celix_properties_set(props, "zone", "a_zone");
 
         rc = endpointDescription_create(props, &endpoint);
@@ -587,7 +590,8 @@ extern "C" {
         celix_properties_set(props, CELIX_RSA_ENDPOINT_ID, "eec5404d-51d0-47ef-8d86-c825a8beda42-42");
         celix_properties_set(props, CELIX_RSA_SERVICE_IMPORTED_CONFIGS, TST_CONFIGURATION_TYPE);
         celix_properties_set(props, CELIX_FRAMEWORK_SERVICE_NAME, "org.apache.celix.test.MyBundle");
-        celix_properties_set(props, "service.version", "1.0.0"); //TODO find out standard in osgi spec
+        auto*v = celix_version_create(1, 0, 0, nullptr);
+        celix_properties_assignVersion(props, "service.version", v);
         celix_properties_set(props, "zone", "a_zone");
 
         rc = endpointDescription_create(props, &endpoint);
@@ -633,7 +637,8 @@ extern "C" {
         celix_properties_set(props, CELIX_RSA_ENDPOINT_ID, "eec5404d-51d0-47ef-8d86-c825a8beda42-42");
         celix_properties_set(props, CELIX_RSA_SERVICE_IMPORTED_CONFIGS, TST_CONFIGURATION_TYPE);
         celix_properties_set(props, CELIX_FRAMEWORK_SERVICE_NAME, "org.apache.celix.test.MyBundle");
-        celix_properties_set(props, "service.version", "1.0.0");
+        auto*v = celix_version_create(1, 0, 0, nullptr);
+        celix_properties_assignVersion(props, "service.version", v);
         celix_properties_set(props, "zone", "a_zone");
 
         rc = endpointDescription_create(props, &endpoint);

libs/framework/src/bundle_archive.c

@@ -62,35 +62,46 @@ struct bundleArchive {
 static celix_status_t celix_bundleArchive_storeBundleStateProperties(bundle_archive_pt archive) {
     bool needUpdate = false;
     celix_autoptr(celix_properties_t) bundleStateProperties = NULL;
-    celix_status_t status = celix_properties_load(archive->savedBundleStatePropertiesPath, 0, &bundleStateProperties);
-    if (status != CELIX_SUCCESS) {
-        celix_framework_logTssErrors(archive->fw->logger, CELIX_LOG_LEVEL_ERROR);
+    if (celix_utils_fileExists(archive->savedBundleStatePropertiesPath)) {
+        celix_status_t status =
+            celix_properties_load(archive->savedBundleStatePropertiesPath, 0, &bundleStateProperties);
+        if (status != CELIX_SUCCESS) {
+            celix_framework_logTssErrors(archive->fw->logger, CELIX_LOG_LEVEL_ERROR);
+        }
+    }
+    if (!bundleStateProperties) {
         bundleStateProperties = celix_properties_create();
     }
-    if (bundleStateProperties == NULL) {
+    if (!bundleStateProperties) {
         return CELIX_ENOMEM;
     }
-    //set/update bundle cache state properties
-    if (celix_properties_getAsLong(bundleStateProperties, CELIX_BUNDLE_ARCHIVE_BUNDLE_ID_PROPERTY_NAME, 0) != archive->id) {
+
+    // set/update bundle cache state properties
+    if (celix_properties_getAsLong(bundleStateProperties, CELIX_BUNDLE_ARCHIVE_BUNDLE_ID_PROPERTY_NAME, 0) !=
+        archive->id) {
         celix_properties_setLong(bundleStateProperties, CELIX_BUNDLE_ARCHIVE_BUNDLE_ID_PROPERTY_NAME, archive->id);
         needUpdate = true;
     }
-    if (strcmp(celix_properties_get(bundleStateProperties, CELIX_BUNDLE_ARCHIVE_LOCATION_PROPERTY_NAME, ""), archive->location) != 0) {
+    if (strcmp(celix_properties_get(bundleStateProperties, CELIX_BUNDLE_ARCHIVE_LOCATION_PROPERTY_NAME, ""),
+               archive->location) != 0) {
         celix_properties_set(bundleStateProperties, CELIX_BUNDLE_ARCHIVE_LOCATION_PROPERTY_NAME, archive->location);
         needUpdate = true;
     }
-    if (strcmp(celix_properties_get(bundleStateProperties, CELIX_BUNDLE_ARCHIVE_SYMBOLIC_NAME_PROPERTY_NAME, ""), archive->bundleSymbolicName) != 0) {
-        celix_properties_set(bundleStateProperties, CELIX_BUNDLE_ARCHIVE_SYMBOLIC_NAME_PROPERTY_NAME, archive->bundleSymbolicName);
+    if (strcmp(celix_properties_get(bundleStateProperties, CELIX_BUNDLE_ARCHIVE_SYMBOLIC_NAME_PROPERTY_NAME, ""),
+               archive->bundleSymbolicName) != 0) {
+        celix_properties_set(
+            bundleStateProperties, CELIX_BUNDLE_ARCHIVE_SYMBOLIC_NAME_PROPERTY_NAME, archive->bundleSymbolicName);
         needUpdate = true;
     }
-    if (strcmp(celix_properties_get(bundleStateProperties, CELIX_BUNDLE_ARCHIVE_VERSION_PROPERTY_NAME, ""), archive->bundleVersion) != 0) {
+    if (strcmp(celix_properties_get(bundleStateProperties, CELIX_BUNDLE_ARCHIVE_VERSION_PROPERTY_NAME, ""),
+               archive->bundleVersion) != 0) {
         celix_properties_set(bundleStateProperties, CELIX_BUNDLE_ARCHIVE_VERSION_PROPERTY_NAME, archive->bundleVersion);
         needUpdate = true;
     }
 
-    //save bundle cache state properties
+    // save bundle cache state properties
     if (needUpdate) {
-        status = celix_properties_save(
+        celix_status_t status = celix_properties_save(
             bundleStateProperties, archive->savedBundleStatePropertiesPath, CELIX_PROPERTIES_ENCODE_PRETTY);
         if (status != CELIX_SUCCESS) {
             return status;

libs/framework/src/celix_launcher.c

@@ -33,10 +33,10 @@
 #include "celix_bundle_context.h"
 #include "celix_constants.h"
 #include "celix_err.h"
+#include "celix_file_utils.h"
 #include "celix_framework_factory.h"
 #include "celix_framework_utils.h"
 
-
 #define DEFAULT_CONFIG_FILE "config.properties"
 
 #define CELIX_LAUNCHER_OK_EXIT_CODE 0
@@ -348,8 +348,7 @@ static celix_status_t celix_launcher_loadRuntimeProperties(const char* configFil
     *outConfigProperties = NULL;
     bool loadConfig = configFile != NULL;
     if (!loadConfig) {
-        struct stat buffer;
-        loadConfig = stat(DEFAULT_CONFIG_FILE, &buffer) == 0;
+        loadConfig = celix_utils_fileExists(DEFAULT_CONFIG_FILE);
         configFile = DEFAULT_CONFIG_FILE;
     }
     if (loadConfig) {

libs/utils/error_injector/celix_version/CMakeLists.txt

@@ -23,6 +23,7 @@ target_link_libraries(version_ei PUBLIC Celix::error_injector Celix::utils)
 target_link_options(version_ei INTERFACE
         LINKER:--wrap,celix_version_createVersionFromString
         LINKER:--wrap,celix_version_parse
+        LINKER:--wrap,celix_version_tryParse
         LINKER:--wrap,celix_version_copy
         LINKER:--wrap,celix_version_toString
 )

libs/utils/error_injector/celix_version/include/celix_version_ei.h

@@ -29,6 +29,8 @@ CELIX_EI_DECLARE(celix_version_createVersionFromString, celix_version_t*);
 
 CELIX_EI_DECLARE(celix_version_parse, celix_status_t);
 
+CELIX_EI_DECLARE(celix_version_tryParse, celix_status_t);
+
 CELIX_EI_DECLARE(celix_version_copy, celix_version_t*);
 
 CELIX_EI_DECLARE(celix_version_toString, char*);

libs/utils/error_injector/celix_version/src/celix_version_ei.cc

@@ -34,6 +34,13 @@ celix_status_t __wrap_celix_version_parse(const char* versionStr, celix_version_
     return __real_celix_version_parse(versionStr, version);
 }
 
+celix_status_t __real_celix_version_tryParse(const char* versionStr, celix_version_t** version);
+CELIX_EI_DEFINE(celix_version_tryParse, celix_status_t)
+celix_status_t __wrap_celix_version_tryParse(const char* versionStr, celix_version_t** version) {
+    CELIX_EI_IMPL(celix_version_tryParse);
+    return __real_celix_version_tryParse(versionStr, version);
+}
+
 celix_version_t* __real_celix_version_copy(const celix_version_t* version);
 CELIX_EI_DEFINE(celix_version_copy, celix_version_t*)
 celix_version_t* __wrap_celix_version_copy(const celix_version_t* version) {

libs/utils/gtest/src/ConvertUtilsErrorInjectionTestSuite.cc

@@ -30,6 +30,7 @@ class ConvertUtilsWithErrorInjectionTestSuite : public ::testing::Test {
     ~ConvertUtilsWithErrorInjectionTestSuite() override {
         celix_ei_expect_celix_version_copy(nullptr, 0, nullptr);
         celix_ei_expect_celix_version_createVersionFromString(nullptr, 0, CELIX_SUCCESS);
+        celix_ei_expect_celix_version_parse(nullptr, 0, CELIX_SUCCESS);
         celix_ei_expect_celix_arrayList_createWithOptions(nullptr, 0, nullptr);
         celix_ei_expect_celix_arrayList_createLongArray(nullptr, 0, nullptr);
         celix_ei_expect_celix_arrayList_addLong(nullptr, 0, CELIX_SUCCESS);

libs/utils/gtest/src/ConvertUtilsTestSuite.cc

@@ -235,7 +235,7 @@ TEST_F(ConvertUtilsTestSuite, ConvertToVersionTest) {
     convertStatus = celix_utils_convertStringToVersion("A", nullptr, &result);
     EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, convertStatus);
     EXPECT_EQ(nullptr, result);
-    EXPECT_STREQ("Invalid version component(0)", celix_err_popLastError());
+    EXPECT_NE(nullptr, strstr(celix_err_popLastError(), "Invalid version part 0."));
 
     //test for a string with a number
     convertStatus = celix_utils_convertStringToVersion("1.2.3.A", nullptr, &result);
@@ -271,13 +271,13 @@ TEST_F(ConvertUtilsTestSuite, ConvertToVersionTest) {
     EXPECT_NE(nullptr, result);
     checkVersion(result, 1, 2, 3, "B"); //default version
     celix_version_destroy(result);
-    EXPECT_STREQ("Invalid version component(0)", celix_err_popLastError());
+    EXPECT_NE(nullptr, strstr(celix_err_popLastError(), "Invalid version part 0."));
 
     //test for a convert with a version value with trailing chars
     convertStatus = celix_utils_convertStringToVersion("2.1.1 and something else", nullptr, &result);
     EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, convertStatus);
     EXPECT_EQ(nullptr, result);
-    EXPECT_STREQ("Invalid trailing string:< and something else>", celix_err_popLastError());
+    EXPECT_NE(nullptr, strstr(celix_err_popLastError(), "Invalid trailing string"));
 
     //test for a convert with a version value with trailing whitespaces
     convertStatus = celix_utils_convertStringToVersion("1.2.3 \t\n", nullptr, &result);

libs/utils/gtest/src/VersionTestSuite.cc

@@ -279,18 +279,18 @@ TEST_F(VersionTestSuite, ParseTest) {
     parseStatus = celix_version_parse(largeLong.c_str(), &result);
     EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, parseStatus);
     EXPECT_EQ(nullptr, result);
-    EXPECT_STREQ("Invalid version component(0)", celix_err_popLastError());
+    EXPECT_TRUE(strstr(celix_err_popLastError(), "Invalid version part 0."));
 
     auto largeInteger = std::to_string(std::numeric_limits<unsigned int>::max());
     parseStatus = celix_version_parse(largeInteger.c_str(), &result);
     EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, parseStatus);
     EXPECT_EQ(nullptr, result);
-    EXPECT_STREQ("Invalid version component(0)", celix_err_popLastError());
+    EXPECT_TRUE(strstr(celix_err_popLastError(), "Invalid version part 0."));
 
     parseStatus = celix_version_parse("", &result);
     EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, parseStatus);
     EXPECT_EQ(nullptr, result);
-    EXPECT_STREQ("Invalid version component(0)", celix_err_popLastError());
+    EXPECT_TRUE(strstr(celix_err_popLastError(), "Invalid version part 0."));
 
     parseStatus = celix_version_parse(nullptr, &result);
     EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, parseStatus);
@@ -299,22 +299,27 @@ TEST_F(VersionTestSuite, ParseTest) {
     parseStatus = celix_version_parse("invalid", &result);
     EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, parseStatus);
     EXPECT_EQ(nullptr, result);
-    EXPECT_STREQ("Invalid version component(0)", celix_err_popLastError());
+    EXPECT_TRUE(strstr(celix_err_popLastError(), "Invalid version part 0."));
 
     parseStatus = celix_version_parse("-1", &result);
     EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, parseStatus);
     EXPECT_EQ(nullptr, result);
-    EXPECT_STREQ("Invalid version component(0)", celix_err_popLastError());
+    EXPECT_TRUE(strstr(celix_err_popLastError(), "Invalid version part 0."));
 
     parseStatus = celix_version_parse("1.-2", &result);
     EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, parseStatus);
     EXPECT_EQ(nullptr, result);
-    EXPECT_STREQ("Invalid version component(1)", celix_err_popLastError());
+    EXPECT_TRUE(strstr(celix_err_popLastError(), "Invalid version part 1."));
 
     parseStatus = celix_version_parse("1.2.-3", &result);
     EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, parseStatus);
     EXPECT_EQ(nullptr, result);
-    EXPECT_STREQ("Invalid version component(2)", celix_err_popLastError());
+    EXPECT_TRUE(strstr(celix_err_popLastError(), "Invalid version part 2."));
+
+    parseStatus = celix_version_tryParse("1.2.-3", &result);
+    EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, parseStatus);
+    EXPECT_EQ(nullptr, result);
+    EXPECT_EQ(0, celix_err_getErrorCount());
 }
 
 TEST_F(VersionTestSuite, HashTest) {

libs/utils/include/celix_version.h

@@ -126,6 +126,18 @@ CELIX_UTILS_EXPORT celix_version_t* celix_version_createVersionFromString(const
  */
 CELIX_UTILS_EXPORT celix_status_t celix_version_parse(const char *versionStr, celix_version_t** version);
 
+/**
+ * @brief Try to parse a version string into a version object.
+ * Identical to celix_version_parse, but does not log an error if the version string is invalid (does log if memory
+ * could not be allocated).
+ *
+ * @param[in] versionStr The version string to parse.
+ * @param[out] version The parsed version object.
+ * @return CELIX_SUCCESS if the version string was parsed successfully, CELIX_ILLEGAL_ARGUMENT if the version string
+ *         was invalid, or CELIX_ENOMEM if memory could not be allocated.
+ */
+CELIX_UTILS_EXPORT celix_status_t celix_version_tryParse(const char* versionStr, celix_version_t** version);
+
 /**
  * @brief Create empty version "0.0.0".
  */

libs/utils/src/filter.c

@@ -539,7 +539,7 @@ static celix_status_t celix_filter_compile(celix_filter_t* filter) {
         internal->boolValue =
                 celix_utils_convertStringToBool(filter->value, false, &internal->convertedToBool);
 
-        celix_status_t convertStatus = celix_utils_convertStringToVersion(filter->value, NULL, &internal->versionValue);
+        celix_status_t convertStatus = celix_version_tryParse(filter->value, &internal->versionValue);
         if (convertStatus == ENOMEM) {
             return ENOMEM;
         }

libs/utils/src/version.c

@@ -112,7 +112,8 @@ celix_version_t* celix_version_createVersionFromString(const char *versionStr) {
     return version;
 }
 
-celix_status_t celix_version_parse(const char* versionStr, celix_version_t** version) {
+static celix_status_t
+celix_version_parseInternal(const char* versionStr, bool logParseError, celix_version_t** version) {
     *version = NULL;
 
     if (versionStr == NULL) {
@@ -128,16 +129,20 @@ celix_status_t celix_version_parse(const char* versionStr, celix_version_t** ver
         errno = 0;
         long l = strtol(token, &endPtr, 10);
         if (errno != 0 || token == endPtr || l < 0 || l >= INT_MAX) {
-            celix_err_pushf("Invalid version component(%d)", count);
-            return  CELIX_ILLEGAL_ARGUMENT;
+            if (logParseError) {
+                celix_err_pushf("Invalid version part %d. Input str: %s", count, versionStr);
+            }
+            return CELIX_ILLEGAL_ARGUMENT;
         }
         versionsParts[count++] = (int)l;
         if (*endPtr == '.') {
             token = endPtr + 1;
-        } else if (celix_utils_isEndptrEndOfStringOrOnlyContainsWhitespaces(endPtr)){
+        } else if (celix_utils_isEndptrEndOfStringOrOnlyContainsWhitespaces(endPtr)) {
             token = NULL;
         } else {
-            celix_err_pushf("Invalid trailing string:<%s>", endPtr);
+            if (logParseError) {
+                celix_err_pushf("Invalid trailing string `%s`. Input str: %s", endPtr, versionStr);
+            }
             return CELIX_ILLEGAL_ARGUMENT;
         }
     }
@@ -148,6 +153,14 @@ celix_status_t celix_version_parse(const char* versionStr, celix_version_t** ver
     return *version ? CELIX_SUCCESS : (errno == EINVAL ? CELIX_ILLEGAL_ARGUMENT : CELIX_ENOMEM);
 }
 
+celix_status_t celix_version_parse(const char* versionStr, celix_version_t** version) {
+    return celix_version_parseInternal(versionStr, true, version);
+}
+
+celix_status_t celix_version_tryParse(const char* versionStr, celix_version_t** version) {
+    return celix_version_parseInternal(versionStr, false, version);
+}
+
 celix_version_t* celix_version_createEmptyVersion() {
     return celix_version_create(0, 0, 0, NULL);
 }