This is an automated email from the ASF dual-hosted git repository.
pnoltes pushed a commit to branch feature/685-refactor-manifest-format
in repository https://gitbox.apache.org/repos/asf/celix.git
commit a40923049a569dd958355d6c2170ebee7a0e4aa4
Author: Pepijn Noltes <pnol...@apache.org>
AuthorDate: Mon Jul 1 20:09:29 2024 +0200
gh-685: Add manifest validation
---
libs/framework/gtest/src/ManifestTestSuite.cc | 90 ++++++++++++++--
libs/framework/src/celix_manifest.c | 143 ++++++++++++++++++++++++--
libs/framework/src/celix_manifest.h | 49 ++++++---
libs/utils/include/celix_err.h | 22 ++++
libs/utils/include/celix_errno.h | 12 +++
libs/utils/include/celix_properties.h | 3 +-
libs/utils/src/properties.c | 5 +-
7 files changed, 290 insertions(+), 34 deletions(-)
diff --git a/libs/framework/gtest/src/ManifestTestSuite.cc
b/libs/framework/gtest/src/ManifestTestSuite.cc
index 66b4f0fa5..9c6d64135 100644
--- a/libs/framework/gtest/src/ManifestTestSuite.cc
+++ b/libs/framework/gtest/src/ManifestTestSuite.cc
@@ -22,26 +22,100 @@
#include "celix_manifest.h"
#include "celix_properties.h"
+#include "celix_err.h"
class ManifestTestSuite : public ::testing::Test {
+ public:
+ ManifestTestSuite() { celix_err_resetErrors(); }
};
TEST_F(ManifestTestSuite, CreateManifestTest) {
+ //Given a properties set with all the mandatory manifest attributes
celix_properties_t *properties = celix_properties_create();
- celix_properties_set(properties, "key", "value");
+ celix_version_t* v = celix_version_create(2, 0, 0, nullptr);
+ celix_properties_assignVersion(properties, "CELIX_MANIFEST_VERSION", v);
+ celix_properties_set(properties, "CELIX_BUNDLE_VERSION", "1.0.0");
+ celix_properties_set(properties, "CELIX_BUNDLE_NAME", "my_bundle");
+ celix_properties_set(properties, "CELIX_BUNDLE_SYMBOLIC_NAME",
"celix_my_bundle");
+
+ //When creating a manifest from the attributes
celix_autoptr(celix_manifest_t) manifest = nullptr;
celix_status_t status = celix_manifest_create(properties, &manifest);
+
+ //Then the creation is successful
EXPECT_EQ(CELIX_SUCCESS, status);
- EXPECT_EQ(1,
celix_properties_size(celix_manifest_getAttributes(manifest)));
+ EXPECT_EQ(4,
celix_properties_size(celix_manifest_getAttributes(manifest)));
+}
+
+TEST_F(ManifestTestSuite, MissingOrInvalidMandatoryManifestAttributesTest) {
+ //Given an empty properties set
+ celix_properties_t *properties = celix_properties_create();
+
+ //When creating a manifest from the attributes
+ celix_autoptr(celix_manifest_t) manifest = nullptr;
+ celix_status_t status = celix_manifest_create(properties, &manifest);
+
+ //Then the creation fails
+ EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, status);
+
+ //And 4 celix err log entries are logged (4 missing attributes)
+ EXPECT_EQ(celix_err_getErrorCount(), 4);
+ celix_err_printErrors(stdout, "Expect Errors: ", nullptr);
+
+ //Given a properties set with not versions attributes for bundle/manifest
version.
+ properties = celix_properties_create();
+ celix_properties_set(properties, "CELIX_BUNDLE_VERSION", "not a version");
+ celix_properties_setBool(properties, "CELIX_MANIFEST_VERSION", false);
+ celix_properties_set(properties, "CELIX_BUNDLE_NAME", "my_bundle");
+ celix_properties_set(properties, "CELIX_BUNDLE_SYMBOLIC_NAME",
"celix_my_bundle");
+ //When creating a manifest from the attributes
celix_autoptr(celix_manifest_t) manifest2 = nullptr;
- status = celix_manifest_create(nullptr, &manifest2);
- EXPECT_EQ(CELIX_SUCCESS, status);
- EXPECT_EQ(0,
celix_properties_size(celix_manifest_getAttributes(manifest2)));
+ status = celix_manifest_create(properties, &manifest2);
+
+ //Then the creation fails
+ EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, status);
+
+ //And 4 celix err log entries are logged (4x invalid versions)
+ EXPECT_EQ(celix_err_getErrorCount(), 2);
+ celix_err_printErrors(stdout, "Expect Errors: ", nullptr);
+ //Given a properties with an unsupported manifest version
+ properties = celix_properties_create();
+ celix_properties_set(properties, "CELIX_MANIFEST_VERSION", "1.0.0");
//note must be 2.0.*
+ celix_properties_set(properties, "CELIX_BUNDLE_VERSION", "1.0.0");
+ celix_properties_set(properties, "CELIX_BUNDLE_NAME", "my_bundle");
+ celix_properties_set(properties, "CELIX_BUNDLE_SYMBOLIC_NAME",
"celix_my_bundle");
+
+ //When creating a manifest from the attributes
celix_autoptr(celix_manifest_t) manifest3 = nullptr;
- status = celix_manifest_createFromFile("empty.properties", &manifest3);
- EXPECT_EQ(CELIX_SUCCESS, status);
- EXPECT_EQ(0,
celix_properties_size(celix_manifest_getAttributes(manifest3)));
+ status = celix_manifest_create(properties, &manifest2);
+
+ //Then the creation fails
+ EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, status);
+
+ //And 1 celix err log entries is logged
+ EXPECT_EQ(celix_err_getErrorCount(), 1);
+ celix_err_printErrors(stdout, "Expect Errors: ", nullptr);
+
+ //Given a properties set with an invalid private libraries list
+ properties = celix_properties_create();
+ celix_properties_set(properties, "CELIX_MANIFEST_VERSION", "2.0.2");
+ celix_properties_set(properties, "CELIX_BUNDLE_VERSION", "1.0.0");
+ celix_properties_set(properties, "CELIX_BUNDLE_NAME", "my_bundle");
+ celix_properties_set(properties, "CELIX_BUNDLE_SYMBOLIC_NAME",
"celix_my_bundle");
+ celix_properties_setBool(properties, "CELIX_BUNDLE_PRIVATE_LIBRARIES",
true);
+
+ //When creating a manifest from the attributes
+ celix_autoptr(celix_manifest_t) manifest4 = nullptr;
+ status = celix_manifest_create(properties, &manifest2);
+
+ //Then the creation fails
+ EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, status);
+
+ //And 1 celix err log entries is logged
+ EXPECT_EQ(celix_err_getErrorCount(), 1);
+ celix_err_printErrors(stdout, "Expect Errors: ", nullptr);
}
+//TODO check if the mandatory and optional attributes are set and can be
retreived with getters
diff --git a/libs/framework/src/celix_manifest.c
b/libs/framework/src/celix_manifest.c
index 7dc550480..31e5db809 100644
--- a/libs/framework/src/celix_manifest.c
+++ b/libs/framework/src/celix_manifest.c
@@ -19,26 +19,58 @@
#include "celix_manifest.h"
+#include "celix_err.h"
#include "celix_properties.h"
#include "celix_stdlib_cleanup.h"
-#include "celix_err.h"
+#include "celix_utils.h"
+#include "celix_version.h"
+
+// Mandatory manifest attributes
+#define CELIX_MANIFEST_VERSION "CELIX_MANIFEST_VERSION"
+#define CELIX_BUNDLE_SYMBOLIC_NAME "CELIX_BUNDLE_SYMBOLIC_NAME"
+#define CELIX_BUNDLE_VERSION "CELIX_BUNDLE_VERSION"
+#define CELIX_BUNDLE_NAME "CELIX_BUNDLE_NAME"
+
+// Optional manifest attributes
+#define CELIX_BUNDLE_PRIVATE_LIBRARIES "CELIX_BUNDLE_PRIVATE_LIBRARIES"
+#define CELIX_BUNDLE_ACTIVATOR_LIBRARY "CELIX_BUNDLE_ACTIVATOR_LIBRARY"
+#define CELIX_BUNDLE_GROUP "CELIX_BUNDLE_GROUP"
struct celix_manifest{
celix_properties_t* attributes;
+
+ //Mandatory fields
+ celix_version_t* manifestVersion;
+ celix_version_t* bundleVersion;
+ char* symbolicName;
+ char* bundleName;
+
+ //Optional fields
+ char* bundleGroup;
+ char* activatorLibrary;
+ celix_array_list_t* privateLibraries;
};
+/**
+ * @brief Set and validate the provided manifest by checking if all mandatory
attributes are present and of the correct
+ * type and checking if the optional attributes, when present, are of the
correct type.
+ */
+static celix_status_t celix_manifest_setAttributes(celix_manifest_t* manifest);
+
celix_status_t celix_manifest_create(celix_properties_t* attributes,
celix_manifest_t** manifestOut) {
+ if (!attributes) {
+ return CELIX_ILLEGAL_ARGUMENT;
+ }
+
celix_autofree celix_manifest_t* manifest = calloc(1, sizeof(*manifest));
if (!manifest) {
celix_properties_destroy(attributes);
celix_err_push("Failed to allocate memory for manifest");
return ENOMEM;
}
+ manifest->attributes = attributes;
- manifest->attributes = attributes ? attributes : celix_properties_create();
- if (!manifest->attributes) {
- return ENOMEM;
- }
+ CELIX_EPROP(celix_manifest_setAttributes(manifest));
*manifestOut = celix_steal_ptr(manifest);
return CELIX_SUCCESS;
@@ -51,13 +83,22 @@ celix_status_t celix_manifest_createFromFile(const char*
filename, celix_manifes
if (status != CELIX_SUCCESS) {
return status;
}
-
return celix_manifest_create(properties, manifestOut);
}
void celix_manifest_destroy(celix_manifest_t* manifest) {
if (manifest) {
celix_properties_destroy(manifest->attributes);
+
+ free(manifest->symbolicName);
+ free(manifest->bundleName);
+ celix_version_destroy(manifest->manifestVersion);
+ celix_version_destroy(manifest->bundleVersion);
+
+ free(manifest->activatorLibrary);
+ free(manifest->bundleGroup);
+ celix_arrayList_destroy(manifest->privateLibraries);
+
free(manifest);
}
}
@@ -66,6 +107,92 @@ const celix_properties_t*
celix_manifest_getAttributes(celix_manifest_t* manifes
return manifest->attributes;
}
-bool celix_manifest_validate(celix_manifest_t* manifest) {
- return true; //TODO implement
+static celix_status_t celix_manifest_setMandatoryAttributes(celix_manifest_t*
manifest) {
+ const char* symbolicName = celix_properties_get(manifest->attributes,
CELIX_BUNDLE_SYMBOLIC_NAME, NULL);
+ const char* bundleName = celix_properties_get(manifest->attributes,
CELIX_BUNDLE_NAME, NULL);
+
+ celix_autoptr(celix_version_t) manifestVersion = NULL;
+ celix_status_t getVersionStatus =
+ celix_properties_getAsVersion(manifest->attributes,
CELIX_MANIFEST_VERSION, NULL, &manifestVersion);
+ CELIX_ERR_RET_IF_ENOMEM(getVersionStatus);
+
+ celix_autoptr(celix_version_t) bundleVersion = NULL;
+ getVersionStatus = celix_properties_getAsVersion(manifest->attributes,
CELIX_BUNDLE_VERSION, NULL, &bundleVersion);
+ CELIX_ERR_RET_IF_ENOMEM(getVersionStatus);
+
+ celix_status_t status = CELIX_SUCCESS;
+ if (!bundleName) {
+ celix_err_push(CELIX_BUNDLE_NAME " is missing");
+ status = CELIX_ILLEGAL_ARGUMENT;
+ }
+ if (!symbolicName) {
+ celix_err_push(CELIX_BUNDLE_SYMBOLIC_NAME " is missing");
+ status = CELIX_ILLEGAL_ARGUMENT;
+ }
+ if (!manifestVersion) {
+ celix_err_push(CELIX_MANIFEST_VERSION " is missing or not a version");
+ status = CELIX_ILLEGAL_ARGUMENT;
+ }
+ if (!bundleVersion) {
+ celix_err_push(CELIX_BUNDLE_VERSION " is missing or not a version");
+ status = CELIX_ILLEGAL_ARGUMENT;
+ }
+
+ if (manifestVersion && celix_version_compareToMajorMinor(manifestVersion,
2, 0) != 0) {
+ celix_err_push(CELIX_MANIFEST_VERSION " is not 2.0.*");
+ status = CELIX_ILLEGAL_ARGUMENT;
+ }
+
+ if (status == CELIX_SUCCESS) {
+ manifest->symbolicName = celix_utils_strdup(symbolicName);
+ CELIX_ERR_RET_IF_NULL(manifest->symbolicName);
+ manifest->bundleName = celix_utils_strdup(bundleName);
+ CELIX_ERR_RET_IF_NULL(manifest->bundleName);
+ manifest->manifestVersion = celix_steal_ptr(manifestVersion);
+ manifest->bundleVersion = celix_steal_ptr(bundleVersion);
+ }
+
+ return status;
+}
+
+static celix_status_t celix_manifest_setOptionalAttributes(celix_manifest_t*
manifest) {
+ celix_status_t status = CELIX_SUCCESS;
+
+ const char* l = celix_properties_getAsString(manifest->attributes,
CELIX_BUNDLE_ACTIVATOR_LIBRARY, NULL);
+ celix_autofree char* activatorLib = NULL;
+ if (l) {
+ activatorLib = celix_utils_strdup(l);
+ CELIX_ERR_RET_IF_NULL(activatorLib);
+ }
+
+ const char* g = celix_properties_getAsString(manifest->attributes,
CELIX_BUNDLE_GROUP, NULL);
+ celix_autofree char* bundleGroup = NULL;
+ if (g) {
+ bundleGroup = celix_utils_strdup(g);
+ CELIX_ERR_RET_IF_NULL(bundleGroup);
+ }
+
+ celix_autoptr(celix_array_list_t) privateLibraries = NULL;
+ celix_status_t getStatus = celix_properties_getAsStringArrayList(
+ manifest->attributes, CELIX_BUNDLE_ACTIVATOR_LIBRARY, NULL,
&privateLibraries);
+ CELIX_ERR_RET_IF_ENOMEM(getStatus);
+ if (celix_properties_hasKey(manifest->attributes,
CELIX_BUNDLE_PRIVATE_LIBRARIES) && !privateLibraries) {
+ celix_err_pushf(CELIX_BUNDLE_PRIVATE_LIBRARIES " is not a string
array. Got: '%s'",
+ celix_properties_get(manifest->attributes,
CELIX_BUNDLE_PRIVATE_LIBRARIES, NULL));
+ status = CELIX_ILLEGAL_ARGUMENT;
+ }
+
+ if (status == CELIX_SUCCESS) {
+ manifest->activatorLibrary = celix_steal_ptr(activatorLib);
+ manifest->bundleGroup = celix_steal_ptr(bundleGroup);
+ manifest->privateLibraries = celix_steal_ptr(privateLibraries);
+ }
+
+ return status;
+}
+
+static celix_status_t celix_manifest_setAttributes(celix_manifest_t* manifest)
{
+ celix_status_t mStatus = celix_manifest_setMandatoryAttributes(manifest);
+ celix_status_t oStatus = celix_manifest_setOptionalAttributes(manifest);
+ return mStatus != CELIX_SUCCESS ? mStatus : oStatus;
}
\ No newline at end of file
diff --git a/libs/framework/src/celix_manifest.h
b/libs/framework/src/celix_manifest.h
index 1efdd0f6e..8704c645c 100644
--- a/libs/framework/src/celix_manifest.h
+++ b/libs/framework/src/celix_manifest.h
@@ -28,17 +28,46 @@
extern "C" {
#endif
+/**
+ * @file celix_manifest.h
+ * @brief Header file for celix_manifest_t.
+ *
+ *
+ * The manifest consists of a set of attributes, including a set of mandatory
attributes.
+ * A manifest is used to describe the contents of a bundle.
+ *
+ * A manifest must contain the following attributes:
+ * - CELIX_MANIFEST_VERSION, type celix_version_t, the version of the manifest.
+ * - CELIX_BUNDLE_SYMBOLIC_NAME, type string, the symbolic name of the bundle.
+ * - CELIX_BUNDLE_VERSION, type celix_version_t, the version of the bundle.
+ * - CELIX_BUNDLE_NAME, type string, the name of the bundle.
+ *
+ * The manifest may contain the following attributes:
+ * - CELIX_BUNDLE_ACTIVATOR_LIBRARY, type string, the activator library of the
bundle.
+ * - CELIX_BUNDLE_PRIVATE_LIBRARIES, type string array, the private libraries
of the bundle.
+ * - CELIX_BUNDLE_GROUP, type string, the group of the bundle. Helps in
grouping sets of bundles.
+ *
+ * And a manifest may contain any other attributes of any type, this can be
retrieved using
+ * celix_manifest_getAttributes.
+ *
+ * A bundle symbolic name can only contain the following characters:
[a-zA-Z0-9_-:]
+ */
+
+/**
+ * @brief The definition of the celix_manifest_t* abstract data type.
+ */
typedef struct celix_manifest celix_manifest_t;
/**
* Create a new manifest using the provided properties.
*
- * If an error occurs, an error message is logged on celix_err.
+ * If an error occurs, an error message is logged on celix_err and in case of
an CELIX_ILLEGAL_ARGUMENT error, there
+ * can be multiple error messages.
*
* @param[in] properties The properties to use for the manifest. Takes
ownership of the properties.
* @param[out] manifest The created manifest.
- * @return CELIX_SUCCESS if no errors occurred, ENOMEM if memory allocation
failed. In case of an error, the provided
- * attributes are destroyed.
+ * @return CELIX_SUCCESS if no errors occurred, ENOMEM if memory allocation
failed and CELIX_ILLEGAL_ARGUMENT if the
+ * provided attributes is incorrect. In case of an error, the provided
attributes are destroyed.
*/
celix_status_t celix_manifest_create(celix_properties_t* attributes,
celix_manifest_t** manifest);
@@ -65,20 +94,12 @@ void celix_manifest_destroy(celix_manifest_t* manifest);
CELIX_DEFINE_AUTOPTR_CLEANUP_FUNC(celix_manifest_t, celix_manifest_destroy)
/**
- * @brief Get the main attributes of the manifest.
- * The attributes are valid as long as the manifest is valid.
+ * @brief Get the manifest attributes.
+ * The manifest attributes are valid as long as the manifest is valid.
*/
const celix_properties_t* celix_manifest_getAttributes(celix_manifest_t*
manifest);
-/**
- * @brief Validate the provided manifest.
- *
- * Logs an error message on celix_err if the manifest is invalid.
- *
- * @param[in] manifest The manifest to validate.
- * @return true if the manifest is valid, false otherwise.
- */
-bool celix_manifest_validate(celix_manifest_t* manifest);
+//TODO getters for mandatory and optional attributes
#ifdef __cplusplus
}
diff --git a/libs/utils/include/celix_err.h b/libs/utils/include/celix_err.h
index 95ce0d612..d617dcec0 100644
--- a/libs/utils/include/celix_err.h
+++ b/libs/utils/include/celix_err.h
@@ -94,6 +94,28 @@ CELIX_UTILS_EXPORT void celix_err_printErrors(FILE* stream,
const char* prefix,
*/
CELIX_UTILS_EXPORT int celix_err_dump(char* buf, size_t size, const char*
prefix, const char* postfix);
+/*!
+ * Helper macro that return with ENOMEM if the status is ENOMEM and logs an
"<func>: Out of memory" error to celix_err.
+ */
+#define CELIX_ERR_RET_IF_ENOMEM(status)
\
+ do {
\
+ if ((status) == ENOMEM) {
\
+ celix_err_pushf("%s: Out of memory", __func__);
\
+ return status;
\
+ }
\
+ } while (0)
+
+/*!
+ * Helper macro that return with ENOMEM if the arg is NULL and logs an
"<func>: Out of memory" error to celix_err.
+ */
+#define CELIX_ERR_RET_IF_NULL(arg)
\
+ do {
\
+ if ((arg) == NULL) {
\
+ celix_err_pushf("%s: Out of memory", __func__);
\
+ return status;
\
+ }
\
+ } while (0)
+
#ifdef __cplusplus
}
#endif
diff --git a/libs/utils/include/celix_errno.h b/libs/utils/include/celix_errno.h
index 8189192b3..dcab62801 100644
--- a/libs/utils/include/celix_errno.h
+++ b/libs/utils/include/celix_errno.h
@@ -56,6 +56,18 @@ extern "C" {
*/
#define CELIX_DO_IF(status, expr) ((status) == CELIX_SUCCESS) ? (expr) :
(status)
+/*!
+ * Helper macro which helps with error propagation. It evaluates the provided
expression that returns a status and
+ * returns the status if the status is not CELIX_SUCCESS (0). If the status is
CELIX_SUCCESS (0) nothing is done.
+ */
+#define CELIX_EPROP(expr)
\
+ do {
\
+ celix_status_t __status = expr;
\
+ if (__status != CELIX_SUCCESS) {
\
+ return __status;
\
+ }
\
+ } while (0)
+
/*!
* Helper macro which check the current status and executes a goto the
provided label if the
* status is not CELIX_SUCCESS (0)
diff --git a/libs/utils/include/celix_properties.h
b/libs/utils/include/celix_properties.h
index b145fc72d..08a2d773b 100644
--- a/libs/utils/include/celix_properties.h
+++ b/libs/utils/include/celix_properties.h
@@ -450,7 +450,8 @@ CELIX_UTILS_EXPORT const celix_version_t*
celix_properties_getVersion(const celi
* @brief Get a value of a property as a copied Celix version.
*
* 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 value is present, but not a Celix version, this function
will attempt to convert the property value
+ * 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.
*
* @note The caller is responsible for deallocating the memory of the returned
version.
diff --git a/libs/utils/src/properties.c b/libs/utils/src/properties.c
index 3ec19a2f7..f48c1eb31 100644
--- a/libs/utils/src/properties.c
+++ b/libs/utils/src/properties.c
@@ -616,9 +616,8 @@ celix_status_t celix_properties_getAsVersion(const
celix_properties_t* propertie
}
*version = copy;
return CELIX_SUCCESS;
- }
- if (entry != NULL && entry->valueType ==
CELIX_PROPERTIES_VALUE_TYPE_STRING) {
- celix_status_t parseStatus = celix_version_parse(entry->value,
version);
+ } else if (entry != NULL) {
+ celix_status_t parseStatus = celix_version_tryParse(entry->value,
version);
if (parseStatus != CELIX_ILLEGAL_ARGUMENT) {
return parseStatus;
}
