gh-685: Improve manifest json impl based on review comments

apache · Nov 10, 2024 · d10a983 · d10a983
1 parent 3048050
commit d10a983
Show file tree

Hide file tree

Showing 17 changed files with 175 additions and 366 deletions.
diff --git a/cmake/cmake_celix/BundlePackaging.cmake b/cmake/cmake_celix/BundlePackaging.cmake
@@ -176,7 +176,7 @@ function(add_celix_bundle)
 
     set(OPTIONS NO_ACTIVATOR DO_NOT_CONFIGURE_SYMBOL_VISIBILITY)
     set(ONE_VAL_ARGS VERSION ACTIVATOR SYMBOLIC_NAME NAME DESCRIPTION FILENAME GROUP)
-    set(MULTI_VAL_ARGS SOURCES PRIVATE_LIBRARIES EXPORT_LIBRARIES IMPORT_LIBRARIES HEADERS)
+    set(MULTI_VAL_ARGS SOURCES PRIVATE_LIBRARIES HEADERS)
     cmake_parse_arguments(BUNDLE "${OPTIONS}" "${ONE_VAL_ARGS}" "${MULTI_VAL_ARGS}" ${ARGN})
 
     ##check arguments
@@ -341,8 +341,6 @@ function(add_celix_bundle)
     #headers
     set_target_properties(${BUNDLE_TARGET_NAME} PROPERTIES "BUNDLE_ACTIVATOR" "") #Library containing the activator (if any)
     set_target_properties(${BUNDLE_TARGET_NAME} PROPERTIES "BUNDLE_PRIVATE_LIBS" "") #List of private libs. 
-    set_target_properties(${BUNDLE_TARGET_NAME} PROPERTIES "BUNDLE_IMPORT_LIBS" "") #List of libs to import
-    set_target_properties(${BUNDLE_TARGET_NAME} PROPERTIES "BUNDLE_EXPORT_LIBS" "") #list of libs to export
     set_target_properties(${BUNDLE_TARGET_NAME} PROPERTIES "BUNDLE_LIB_TARGETS" "") #list of all lib targets built within the project.
     set_target_properties(${BUNDLE_TARGET_NAME} PROPERTIES "BUNDLE_HEADERS" "") #Additional headers will be added (new line seperated) to the manifest
     ################################
@@ -379,22 +377,9 @@ function(add_celix_bundle)
 
 
     celix_bundle_private_libs(${BUNDLE_TARGET_NAME} ${BUNDLE_PRIVATE_LIBRARIES})
-    celix_bundle_export_libs(${BUNDLE_TARGET_NAME} ${BUNDLE_EXPORT_LIBRARIES})
-    celix_bundle_import_libs(${BUNDLE_TARGET_NAME} ${BUNDLE_IMPORT_LIBRARIES})
     celix_bundle_headers(${BUNDLE_TARGET_NAME} ${BUNDLE_HEADERS})
 endfunction()
 
-#[[
-Adds a export lib to the Celix bundle.
-
-NOTE: Currently export lib support is Celix is not complete and still experimental.
-]]
-function(celix_bundle_export_libs)
-    list(GET ARGN 0 BUNDLE)
-    list(REMOVE_AT ARGN 0)
-    celix_bundle_libs(${BUNDLE} "EXPORT" TRUE ${ARGN})
-endfunction()
-
 #[[
 Add libraries to a bundle.
 
@@ -527,37 +512,6 @@ function(celix_bundle_libs)
     set_target_properties(${BUNDLE} PROPERTIES "BUNDLE_LIB_TARGETS" "${LIB_TARGETS}")
 endfunction()
 
-#[[
-Adds a import lib to the Celix bundle.
-
-NOTE: Currently importing lib support is Celix is not complete and still experimental.
-]]
-function(celix_bundle_import_libs)
-    #0 is bundle TARGET
-    #2..n is import libs
-    list(GET ARGN 0 BUNDLE)
-    list(REMOVE_AT ARGN 0)
-
-    #check if arg 0 is correct
-    _check_bundle(${BUNDLE})
-
-    get_target_property(LIBS ${BUNDLE} "BUNDLE_IMPORT_LIBS")
-
-    foreach (LIB IN ITEMS ${ARGN})
-        message(WARNING "Bundle with import libs in Celix is not complete and still experimental.")
-        if (IS_ABSOLUTE ${LIB} AND EXISTS ${LIB})
-            list(APPEND LIBS ${LIB_NAME})
-        else ()
-            list(APPEND LIBS "$<TARGET_SONAME_FILE_NAME:${LIB}>")
-        endif ()
-
-        target_link_libraries(${BUNDLE} PRIVATE ${LIB})
-    endforeach ()
-
-
-    set_target_properties(${BUNDLE} PROPERTIES "BUNDLE_IMPORT_LIBS" "${LIBS}")
-endfunction()
-
 #[[
 Add files to the target bundle.
 

diff --git a/cmake/cmake_celix/templates/MANIFEST.json.in b/cmake/cmake_celix/templates/MANIFEST.json.in
@@ -1,14 +1,12 @@
 {
     $<JOIN:$<TARGET_PROPERTY:${BUNDLE_TARGET_NAME},BUNDLE_HEADERS>,$<COMMA>
     >$<$<BOOL:$<TARGET_PROPERTY:${BUNDLE_TARGET_NAME},BUNDLE_HEADERS>>:$<COMMA>>
-    "CELIX_BUNDLE_MANIFEST_VERSION" : "2.0.0",
     "CELIX_BUNDLE_SYMBOLIC_NAME" : "$<TARGET_PROPERTY:${BUNDLE_TARGET_NAME},BUNDLE_SYMBOLIC_NAME>",
-    "CELIX_BUNDLE_VERSION" : "$<TARGET_PROPERTY:${BUNDLE_TARGET_NAME},BUNDLE_VERSION>",
+    "CELIX_BUNDLE_VERSION" : "version<$<TARGET_PROPERTY:${BUNDLE_TARGET_NAME},BUNDLE_VERSION>$<ANGLE-R>",
     "CELIX_BUNDLE_NAME" : "$<TARGET_PROPERTY:${BUNDLE_TARGET_NAME},BUNDLE_NAME>",
-    "CELIX_BUNDLE_ACTIVATOR_LIBRARY": "$<TARGET_PROPERTY:${BUNDLE_TARGET_NAME},BUNDLE_ACTIVATOR>",
-    "CELIX_BUNDLE_PRIVATE_LIBRARIES" : "$<JOIN:$<TARGET_PROPERTY:${BUNDLE_TARGET_NAME},BUNDLE_PRIVATE_LIBS>,$<COMMA>>",
-    "CELIX_BUNDLE_DESCRIPTION" : "$<TARGET_PROPERTY:${BUNDLE_TARGET_NAME},BUNDLE_DESCRIPTION>",
-    "CELIX_BUNDLE_GROUP" : "$<TARGET_PROPERTY:${BUNDLE_TARGET_NAME},BUNDLE_GROUP>",
-    "CELIX_BUNDLE_IMPORT_LIBRARIES" : "$<JOIN:$<TARGET_PROPERTY:${BUNDLE_TARGET_NAME},BUNDLE_IMPORT_LIBS>,$<COMMA>>",
-    "CELIX_BUNDLE_EXPORT_LIBRARIES" : "$<JOIN:$<TARGET_PROPERTY:${BUNDLE_TARGET_NAME},BUNDLE_EXPORT_LIBS>,$<COMMA>>"
+    $<$<BOOL:$<TARGET_PROPERTY:${BUNDLE_TARGET_NAME},BUNDLE_ACTIVATOR>>:"CELIX_BUNDLE_ACTIVATOR_LIBRARY": "$<TARGET_PROPERTY:${BUNDLE_TARGET_NAME},BUNDLE_ACTIVATOR>",>
+    $<$<BOOL:$<TARGET_PROPERTY:${BUNDLE_TARGET_NAME},BUNDLE_PRIVATE_LIBS>>:"CELIX_BUNDLE_PRIVATE_LIBRARIES" : [$<JOIN:"$<TARGET_PROPERTY:${BUNDLE_TARGET_NAME},BUNDLE_PRIVATE_LIBS>","$<COMMA>">],>
+    $<$<BOOL:$<TARGET_PROPERTY:${BUNDLE_TARGET_NAME},BUNDLE_DESCRIPTION>>:"CELIX_BUNDLE_DESCRIPTION" : "$<TARGET_PROPERTY:${BUNDLE_TARGET_NAME},BUNDLE_DESCRIPTION>",>
+    $<$<BOOL:$<TARGET_PROPERTY:${BUNDLE_TARGET_NAME},BUNDLE_GROUP>>:"CELIX_BUNDLE_GROUP" : "$<TARGET_PROPERTY:${BUNDLE_TARGET_NAME},BUNDLE_GROUP>",>
+    "CELIX_BUNDLE_MANIFEST_VERSION" : "version<2.0.0$<ANGLE-R>"
 }
diff --git a/libs/framework/gtest/src/CxxBundleContextTestSuite.cc b/libs/framework/gtest/src/CxxBundleContextTestSuite.cc
@@ -710,8 +710,8 @@ TEST_F(CxxBundleContextTestSuite, GetBundleInformation) {
             EXPECT_EQ(bnd.getGroup(), std::string{"test/group"});
             EXPECT_EQ(bnd.getDescription(), std::string{"Test Description"});
             EXPECT_TRUE(strstr(bnd.getLocation().c_str(), ".zip") != nullptr);
-            EXPECT_TRUE(!bnd.getEntry("META-INF/MANIFEST.MF").empty());
-            EXPECT_EQ(bnd.getEntry("/META-INF/MANIFEST.MF"), bnd.getEntry("META-INF/MANIFEST.MF"));
+            EXPECT_TRUE(!bnd.getEntry("META-INF/MANIFEST.json").empty());
+            EXPECT_EQ(bnd.getEntry("/META-INF/MANIFEST.json"), bnd.getEntry("META-INF/MANIFEST.json"));
             EXPECT_EQ(bnd.getEntry("does-not-exist"), std::string{});
             EXPECT_EQ(bnd.getManifestValue("Extra-Header1"), std::string{"value1"});
             EXPECT_EQ(bnd.getManifestValue("non-existing"), std::string{});

diff --git a/libs/framework/gtest/src/ManifestTestSuite.cc b/libs/framework/gtest/src/ManifestTestSuite.cc
@@ -25,6 +25,7 @@
 #include "celix_properties.h"
 #include "celix_stdlib_cleanup.h"
 #include "celix_framework_version.h"
+#include "celix_version.h"
 
 class ManifestTestSuite : public ::testing::Test {
   public:
@@ -35,8 +36,10 @@ class ManifestTestSuite : public ::testing::Test {
                                                 const char* bundleName,
                                                 const char* symbolicName) {
         celix_properties_t* properties = celix_properties_create();
-        celix_properties_set(properties, "CELIX_BUNDLE_MANIFEST_VERSION", manifestVersion);
-        celix_properties_set(properties, "CELIX_BUNDLE_VERSION", bundleVersion);
+        auto mVer = celix_version_createVersionFromString(manifestVersion);
+        auto bVer = celix_version_createVersionFromString(bundleVersion);
+        celix_properties_assignVersion(properties, "CELIX_BUNDLE_MANIFEST_VERSION", mVer);
+        celix_properties_assignVersion(properties, "CELIX_BUNDLE_VERSION", bVer);
         celix_properties_set(properties, "CELIX_BUNDLE_NAME", bundleName);
         celix_properties_set(properties, "CELIX_BUNDLE_SYMBOLIC_NAME", symbolicName);
         return properties;
@@ -46,9 +49,10 @@ class ManifestTestSuite : public ::testing::Test {
 TEST_F(ManifestTestSuite, CreateManifestTest) {
     //Given a properties set with all the mandatory manifest attributes
     celix_properties_t *properties = celix_properties_create();
-    celix_version_t* v = celix_version_create(2, 0, 0, nullptr);
+    auto* v = celix_version_create(2, 0, 0, nullptr);
     celix_properties_assignVersion(properties, "CELIX_BUNDLE_MANIFEST_VERSION", v);
-    celix_properties_set(properties, "CELIX_BUNDLE_VERSION", "1.0.0");
+    auto* bv = celix_version_create(1, 0, 0, nullptr);
+    celix_properties_assignVersion(properties, "CELIX_BUNDLE_VERSION", bv);
     celix_properties_set(properties, "CELIX_BUNDLE_NAME", "my_bundle");
     celix_properties_set(properties, "CELIX_BUNDLE_SYMBOLIC_NAME", "celix_my_bundle");
 
@@ -79,7 +83,7 @@ TEST_F(ManifestTestSuite, MissingOrInvalidMandatoryManifestAttributesTest) {
     celix_status_t status = celix_bundleManifest_create(properties, &manifest);
 
     //Then the creation fails
-    EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, status);
+    EXPECT_EQ(CELIX_INVALID_SYNTAX, status);
 
     //And 4 celix err log entries are logged (4 missing attributes)
     EXPECT_EQ(celix_err_getErrorCount(), 4);
@@ -97,7 +101,7 @@ TEST_F(ManifestTestSuite, MissingOrInvalidMandatoryManifestAttributesTest) {
     status = celix_bundleManifest_create(properties, &manifest2);
 
     //Then the creation fails
-    EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, status);
+    EXPECT_EQ(CELIX_INVALID_SYNTAX, status);
 
     //And 4 celix err log entries are logged (4x invalid versions)
     EXPECT_EQ(celix_err_getErrorCount(), 2);
@@ -111,7 +115,7 @@ TEST_F(ManifestTestSuite, MissingOrInvalidMandatoryManifestAttributesTest) {
     status = celix_bundleManifest_create(properties, &manifest3);
 
     //Then the creation fails
-    EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, status);
+    EXPECT_EQ(CELIX_INVALID_SYNTAX, status);
 
     //And 1 celix err log entries is logged
     EXPECT_EQ(celix_err_getErrorCount(), 1);
@@ -126,7 +130,7 @@ TEST_F(ManifestTestSuite, MissingOrInvalidMandatoryManifestAttributesTest) {
     status = celix_bundleManifest_create(properties, &manifest4);
 
     //Then the creation fails
-    EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, status);
+    EXPECT_EQ(CELIX_INVALID_SYNTAX, status);
 
     //And 1 celix err log entries is logged
     EXPECT_EQ(celix_err_getErrorCount(), 1);
@@ -142,7 +146,7 @@ TEST_F(ManifestTestSuite, InvalidBundleSymbolicNameTest) {
         celix_status_t status = celix_bundleManifest_create(properties, &manifest);
 
         //Then the creation fails
-        EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, status);
+        EXPECT_EQ(CELIX_INVALID_SYNTAX, status);
 
         //And 1 celix err log entries is logged
         EXPECT_EQ(celix_err_getErrorCount(), 1);
@@ -184,7 +188,10 @@ TEST_F(ManifestTestSuite, GetBuiltinAttributes) {
         //Given a properties set with all the mandatory and optional attributes
         properties = createAttributes("2.0.0", "1.0.0", "my_bundle", "celix_my_bundle");
         celix_properties_set(properties, "CELIX_BUNDLE_ACTIVATOR_LIBRARY", "my_activator");
-        celix_properties_set(properties, "CELIX_BUNDLE_PRIVATE_LIBRARIES", "lib1,lib2");
+        auto* libs = celix_arrayList_createStringArray();
+        celix_arrayList_addString(libs, "lib1");
+        celix_arrayList_addString(libs, "lib2");
+        celix_properties_assignArrayList(properties, "CELIX_BUNDLE_PRIVATE_LIBRARIES", libs);
         celix_properties_set(properties, "CELIX_BUNDLE_GROUP", "my_group");
         celix_properties_set(properties, "CELIX_BUNDLE_DESCRIPTION", "my_description");
 

diff --git a/libs/framework/gtest/src/activator_with_exception.c b/libs/framework/gtest/src/activator_with_exception.c
@@ -19,6 +19,7 @@
 
 #include "celix_bundle_activator.h"
 #include "celix_compiler.h"
+#include <stdio.h>
 
 struct bundle_act {
 

diff --git a/libs/framework/include_deprecated/bundle_context.h b/libs/framework/include_deprecated/bundle_context.h
@@ -28,8 +28,6 @@
 
 #include <stdlib.h>
 
-#include "bundle_context.h"
-
 #include "celix_types.h"
 #include "celix_cleanup.h"
 #include "service_factory.h"

diff --git a/libs/framework/src/bundle.c b/libs/framework/src/bundle.c
@@ -17,23 +17,24 @@
  * under the License.
  */
 
-#include "celix_bundle_private.h"
-
-#include <stdlib.h>
-#include <string.h>
-#include <service_tracker.h>
-#include <celix_constants.h>
-#include <assert.h>
-#include <unistd.h>
+#include "bundle_revision_private.h"
 
+#include "celix_bundle_manifest.h"
+#include "celix_bundle_private.h"
 #include "celix_module.h"
+#include "celix_properties.h"
+#include "celix_properties_type.h"
 #include "framework_private.h"
 #include "utils.h"
 #include "celix_file_utils.h"
 #include "bundle_archive_private.h"
 #include "bundle_context_private.h"
 #include "service_tracker_private.h"
 
+#include <stdlib.h>
+#include <string.h>
+#include <assert.h>
+#include <unistd.h>
 
 static char* celix_bundle_getBundleOrPersistentStoreEntry(const celix_bundle_t* bnd, bool bundleEntry, const char* name);
 celix_status_t bundle_createModule(bundle_pt bundle, celix_module_t** module);
@@ -56,10 +57,6 @@ celix_status_t celix_bundle_createFromArchive(celix_framework_t *framework, bund
     bundle->handle = NULL;
     bundle->activator = NULL;
     bundle->context = NULL;
-    bundle->symbolicName = NULL;
-    bundle->name = NULL;
-    bundle->group = NULL;
-    bundle->description = NULL;
 
     if (bundle->modules == NULL) {
         status = CELIX_ENOMEM;
@@ -90,10 +87,6 @@ celix_status_t bundle_destroy(bundle_pt bundle) {
     }
     celix_arrayList_destroy(bundle->modules);
 
-    free(bundle->symbolicName);
-    free(bundle->name);
-    free(bundle->group);
-    free(bundle->description);
     free(bundle);
 
     return CELIX_SUCCESS;
@@ -158,14 +151,8 @@ celix_status_t bundle_setState(bundle_pt bundle, bundle_state_e state) {
 
 celix_status_t bundle_createModule(bundle_pt bundle, celix_module_t** moduleOut) {
     celix_status_t status = CELIX_SUCCESS;
-    long bundleId = celix_bundle_getId(bundle);
 
-    celix_module_t* module = NULL;
-    if (bundleId == CELIX_FRAMEWORK_BUNDLE_ID) {
-        module = module_createFrameworkModule(bundle->framework, bundle);
-    } else {
-        module = module_create(bundle);
-    }
+    celix_module_t* module = module_create(bundle);
     if (!module) {
         status = CELIX_BUNDLE_EXCEPTION;
         fw_logCode(bundle->framework->logger, CELIX_LOG_LEVEL_ERROR, status, "Cannot create module.");
@@ -199,14 +186,7 @@ celix_status_t bundle_createModule(bundle_pt bundle, celix_module_t** moduleOut)
 }
 
 celix_status_t bundle_addModule(bundle_pt bundle, celix_module_t* module) {
-	celix_arrayList_add(bundle->modules, module);
-
-    //free previous module info
-    free(bundle->symbolicName);
-    free(bundle->name);
-    free(bundle->group);
-    free(bundle->description);
-
+    celix_arrayList_add(bundle->modules, module);
 
     //set new module info
     const char *sn = NULL;
@@ -217,12 +197,8 @@ celix_status_t bundle_addModule(bundle_pt bundle, celix_module_t* module) {
     module_getGroup(module, &g);
     module_getName(module, &n);
     module_getDescription(module, &d);
-    bundle->symbolicName = celix_utils_strdup(sn);
-    bundle->name = celix_utils_strdup(n);
-    bundle->group = celix_utils_strdup(g);
-    bundle->description = celix_utils_strdup(d);
 
-	return CELIX_SUCCESS;
+    return CELIX_SUCCESS;
 }
 
 celix_status_t bundle_isSystemBundle(const_bundle_pt bundle, bool *systemBundle) {
@@ -361,40 +337,37 @@ char* celix_bundle_getDataFile(const celix_bundle_t* bnd, const char *path) {
     return entry;
 }
 
+static celix_bundle_manifest_t* celix_bundle_getManifest(const celix_bundle_t* bnd) {
+    celix_bundle_manifest_t* man = celix_bundleArchive_getManifest(bnd->archive);
+    assert(man); //bundles always have a manifest
+    return man;
+}
+
 const char* celix_bundle_getManifestValue(const celix_bundle_t* bnd, const char* attribute) {
-    const char* header = NULL;
-    if (bnd != NULL) {
-        bundle_archive_t* arch = NULL;
-        bundle_getArchive(bnd, &arch);
-        if (arch != NULL) {
-            celix_bundle_revision_t* rev = NULL;
-            bundleArchive_getCurrentRevision(arch, &rev);
-            if (rev != NULL) {
-                celix_bundle_manifest_t* man = celix_bundleRevision_getManifest(rev);
-                if (man != NULL) {
-                    const celix_properties_t* attr = celix_bundleManifest_getAttributes(man);
-                    header = celix_properties_getAsString(attr, attribute, NULL);
-                }
-            }
-        }
-    }
-    return header;
+    celix_bundle_manifest_t* man = celix_bundle_getManifest(bnd);
+    assert(man); //bundle always has a manifest
+    const celix_properties_t* attr = celix_bundleManifest_getAttributes(man);
+    return celix_properties_getAsString(attr, attribute, NULL);
 }
 
 const char* celix_bundle_getGroup(const celix_bundle_t *bnd) {
-	return bnd->group;
+    celix_bundle_manifest_t* man = celix_bundle_getManifest(bnd);
+    return celix_bundleManifest_getBundleGroup(man);
 }
 
 const char* celix_bundle_getSymbolicName(const celix_bundle_t *bnd) {
-	return bnd->symbolicName;
+    celix_bundle_manifest_t* man = celix_bundle_getManifest(bnd);
+    return celix_bundleManifest_getBundleSymbolicName(man);
 }
 
 const char* celix_bundle_getName(const celix_bundle_t* bnd) {
-    return bnd->name;
+    celix_bundle_manifest_t* man = celix_bundle_getManifest(bnd);
+    return celix_bundleManifest_getBundleName(man);
 }
 
 const char* celix_bundle_getDescription(const celix_bundle_t* bnd) {
-    return bnd->description;
+    celix_bundle_manifest_t* man = celix_bundle_getManifest(bnd);
+    return celix_bundleManifest_getBundleDescription(man);
 }
 
 char* celix_bundle_getLocation(const celix_bundle_t *bnd) {