gh-685: Remove bundle revision and refactor bundle archive

Author: pnoltes

libs/framework/CMakeLists.txt

@@ -23,8 +23,8 @@ if (FRAMEWORK)
         find_package(CURL REQUIRED)
     endif ()
     set(FRAMEWORK_SRC
-            src/bundle.c src/bundle_archive.c src/celix_bundle_cache.c
-            src/bundle_context.c src/bundle_revision.c
+            src/bundle.c src/celix_bundle_archive.c src/celix_bundle_cache.c
+            src/bundle_context.c
             src/framework.c
             src/module.c
             src/service_reference.c src/service_registration.c

libs/framework/gtest/src/BundleArchiveTestSuite.cc

@@ -24,10 +24,9 @@
 #include "celix_file_utils.h"
 #include "celix_framework_utils.h"
 #include "framework.h"
-#include "bundle_archive.h"
 
 //including private headers, which should only be used for testing
-#include "bundle_archive_private.h"
+#include "celix_bundle_archive.h"
 #include "celix_bundle_private.h"
 
 class CxxBundleArchiveTestSuite : public ::testing::Test {

libs/framework/gtest/src/BundleArchiveWithErrorInjectionTestSuite.cc

@@ -32,8 +32,7 @@
 #include "celix/FrameworkFactory.h"
 
 #include "asprintf_ei.h"
-#include "bundle_archive_private.h"
-#include "bundle_revision_private.h"
+#include "celix_bundle_archive.h"
 #include "framework_private.h"
 #include "malloc_ei.h"
 #include "celix_bundle_manifest.h"
@@ -113,19 +112,6 @@ TEST_F(BundleArchiveWithErrorInjectionTestSuite, BundleArchiveCreatedFailedTest)
     // Given a mocked malloc which returns NULL from a call from manifest_create
     celix_ei_expect_calloc((void*)celix_bundleManifest_create, 0, nullptr);
     installBundleAndExpectFailure();
-
-    teardownErrorInjectors();
-    // Given a mocked calloc which returns NULL from a call from bundleRevision_create
-    celix_ei_expect_calloc((void*)celix_bundleRevision_create, 0, nullptr);
-    installBundleAndExpectFailure();
-
-    teardownErrorInjectors();
-    // Given a mocked celix_utils_strdup which returns NULL from a call from bundleRevision_create
-    celix_ei_expect_celix_utils_strdup((void*)celix_bundleRevision_create, 0, nullptr);
-    installBundleAndExpectFailure();
-
-    celix_ei_expect_celix_utils_strdup((void*)celix_bundleRevision_create, 0, nullptr, 2);
-    installBundleAndExpectFailure();
 }
 
 TEST_F(BundleArchiveWithErrorInjectionTestSuite, BundleArchiveCreateCacheDirectoryFailedTest) {
@@ -142,13 +128,13 @@ TEST_F(BundleArchiveWithErrorInjectionTestSuite, BundleArchiveCreateCacheDirecto
     installBundleAndExpectFailure();
 
     teardownErrorInjectors();
-    // Given a mocked celix_utils_strdup which returns NULL from a (indirect) call from bundleArchive_create
-    celix_ei_expect_celix_utils_strdup((void*)celix_bundleArchive_create, 1, nullptr);
+    // Given a mocked celix_utils_strdup which returns NULL from a call from bundleArchive_create
+    celix_ei_expect_celix_utils_strdup((void*)celix_bundleArchive_create, 0, nullptr);
     installBundleAndExpectFailure();
 
     teardownErrorInjectors();
-    // Given a mocked celix_utils_strdup which returns NULL from a second (indirect) call from bundleArchive_create
-    celix_ei_expect_celix_utils_strdup((void*)celix_bundleArchive_create, 1, nullptr, 2);
+    // Given a mocked celix_utils_strdup which returns NULL from a second call from bundleArchive_create
+    celix_ei_expect_celix_utils_strdup((void*)celix_bundleArchive_create, 0, nullptr, 2);
     installBundleAndExpectFailure();
 }
 
@@ -189,7 +175,7 @@ class CelixBundleArchiveErrorInjectionTestSuite : public ::testing::Test {
 TEST_F(CelixBundleArchiveErrorInjectionTestSuite, ArchiveCreateErrorTest) {
     celix_bundle_cache_t* cache = nullptr;
     createCache(&cache);
-    bundle_archive_t* archive = nullptr;
+    celix_bundle_archive_t* archive = nullptr;
 
     // archive directory creation failures not covered by other tests
     celix_ei_expect_celix_utils_getLastModified((void*)celix_bundleArchive_create, 2, CELIX_FILE_IO_EXCEPTION);
@@ -241,22 +227,6 @@ TEST_F(CelixBundleArchiveErrorInjectionTestSuite, ArchiveCreateErrorTest) {
     EXPECT_FALSE(celix_utils_directoryExists(TEST_ARCHIVE_ROOT));
     teardownErrorInjectors();
 
-    // revision creation failure
-    celix_ei_expect_calloc((void*)celix_bundleRevision_create, 0, nullptr);
-    EXPECT_EQ(CELIX_ENOMEM,
-              celix_bundleArchive_create(&fw, TEST_ARCHIVE_ROOT, 1, SIMPLE_TEST_BUNDLE1_LOCATION, &archive));
-    EXPECT_EQ(nullptr, archive);
-    EXPECT_FALSE(celix_utils_directoryExists(TEST_ARCHIVE_ROOT));
-    teardownErrorInjectors();
-
-    // bundle state persistence failure
-    celix_ei_expect_celix_properties_create((void*)celix_bundleArchive_create, 1, nullptr);
-    EXPECT_EQ(CELIX_ENOMEM,
-              celix_bundleArchive_create(&fw, TEST_ARCHIVE_ROOT, 1, SIMPLE_TEST_BUNDLE1_LOCATION, &archive));
-    EXPECT_EQ(nullptr, archive);
-    EXPECT_FALSE(celix_utils_directoryExists(TEST_ARCHIVE_ROOT));
-    teardownErrorInjectors();
-
     EXPECT_EQ(CELIX_SUCCESS, celix_bundleCache_destroy(cache));
 }
 

libs/framework/gtest/src/CelixBundleCacheErrorInjectionTestSuite.cc

@@ -32,8 +32,7 @@
 #include "celix_properties_ei.h"
 
 #include "asprintf_ei.h"
-#include "bundle_archive_private.h"
-#include "bundle_revision_private.h"
+#include "celix_bundle_archive.h"
 #include "framework_private.h"
 #include "malloc_ei.h"
 #include "celix_bundle_manifest.h"
@@ -106,7 +105,7 @@ TEST_F(CelixBundleCacheErrorInjectionTestSuite, ArchiveCreateErrorTest) {
     celix_bundle_cache_t* cache = nullptr;
     createCache(&cache);
 
-    bundle_archive_t* archive = nullptr;
+    celix_bundle_archive_t* archive = nullptr;
     celix_ei_expect_celix_utils_writeOrCreateString((void*)celix_bundleCache_createArchive, 0, nullptr);
     EXPECT_EQ(CELIX_ENOMEM, celix_bundleCache_createArchive(cache, 1, SIMPLE_TEST_BUNDLE1_LOCATION, &archive));
     EXPECT_EQ(nullptr, archive);
@@ -131,7 +130,7 @@ TEST_F(CelixBundleCacheErrorInjectionTestSuite, SystemArchiveCreateErrorTest) {
     celix_bundle_cache_t* cache = nullptr;
     createCache(&cache);
 
-    bundle_archive_t* archive = nullptr;
+    celix_bundle_archive_t* archive = nullptr;
     celix_ei_expect_calloc((void*)celix_bundleArchive_create, 0, nullptr);
     EXPECT_EQ(CELIX_ENOMEM, celix_bundleCache_createSystemArchive(&fw, &archive));
     EXPECT_EQ(nullptr, archive);
@@ -148,10 +147,6 @@ TEST_F(CelixBundleCacheErrorInjectionTestSuite, SystemArchiveCreateErrorTest) {
     EXPECT_EQ(CELIX_ENOMEM, celix_bundleCache_createSystemArchive(&fw, &archive));
     EXPECT_EQ(nullptr, archive);
 
-    celix_ei_expect_calloc((void*)celix_bundleRevision_create, 0, nullptr);
-    EXPECT_EQ(CELIX_ENOMEM, celix_bundleCache_createSystemArchive(&fw, &archive));
-    EXPECT_EQ(nullptr, archive);
-
     EXPECT_EQ(CELIX_SUCCESS, celix_bundleCache_destroy(cache));
 }
 

libs/framework/gtest/src/CelixBundleCacheTestSuite.cc

@@ -17,7 +17,7 @@
   under the License.
  */
 
-#include "bundle_archive_private.h"
+#include "celix_bundle_archive.h"
 #include "celix_bundle_cache.h"
 #include "celix_constants.h"
 #include "celix_file_utils.h"
@@ -48,12 +48,11 @@ class CelixBundleCacheTestSuite : public ::testing::Test {
 };
 
 TEST_F(CelixBundleCacheTestSuite, ArchiveCreateDestroyTest) {
-    bundle_archive_t* archive = nullptr;
+    celix_bundle_archive_t* archive = nullptr;
     EXPECT_EQ(CELIX_SUCCESS, celix_bundleCache_createArchive(fw.cache, 1, SIMPLE_TEST_BUNDLE1_LOCATION, &archive));
     EXPECT_NE(nullptr, archive);
     auto location = celix_bundleArchive_getLocation(archive);
     EXPECT_STREQ(SIMPLE_TEST_BUNDLE1_LOCATION, location);
-    free(location);
     long bndId;
     auto status = celix_bundleCache_findBundleIdForLocation(fw.cache, SIMPLE_TEST_BUNDLE1_LOCATION, &bndId);
     EXPECT_EQ(CELIX_SUCCESS, status);
@@ -71,7 +70,7 @@ TEST_F(CelixBundleCacheTestSuite, ArchiveCreateDestroyTest) {
 }
 
 TEST_F(CelixBundleCacheTestSuite, NonPermanentDestroyTest) {
-    bundle_archive_t* archive = nullptr;
+    celix_bundle_archive_t* archive = nullptr;
     EXPECT_EQ(CELIX_SUCCESS, celix_bundleCache_createArchive(fw.cache, 1, SIMPLE_TEST_BUNDLE1_LOCATION, &archive));
     EXPECT_NE(nullptr, archive);
     std::string loc = celix_bundleArchive_getPersistentStoreRoot(archive);
@@ -86,13 +85,11 @@ TEST_F(CelixBundleCacheTestSuite, NonPermanentDestroyTest) {
 }
 
 TEST_F(CelixBundleCacheTestSuite, SystemArchiveCreateDestroyTest) {
-    bundle_archive_t* archive = nullptr;
-    const char* archiveRoot = nullptr;
+    celix_bundle_archive_t* archive = nullptr;
     EXPECT_EQ(CELIX_SUCCESS, celix_bundleCache_createSystemArchive(&fw, &archive));
     EXPECT_NE(nullptr, archive);
     EXPECT_EQ(0, celix_bundleArchive_getId(archive));
-    EXPECT_EQ(CELIX_SUCCESS, bundleArchive_getArchiveRoot(archive, &archiveRoot));
-    EXPECT_EQ(nullptr, archiveRoot);
+    EXPECT_EQ(nullptr, celix_bundleArchive_getArchiveRoot(archive));
     EXPECT_EQ(nullptr, celix_bundleArchive_getLocation(archive));
     celix_bundleArchive_invalidate(archive);
     celix_bundleCache_destroyArchive(fw.cache, archive);

libs/framework/include/celix_types.h

@@ -41,6 +41,7 @@ extern "C" {
 
 typedef struct celix_framework celix_framework_t;
 typedef struct celix_bundle celix_bundle_t;
+typedef struct celix_bundle_archive celix_bundle_archive_t;
 typedef struct celix_dependency_manager celix_dependency_manager_t;
 typedef struct celix_dm_component_struct celix_dm_component_t;
 typedef struct celix_dm_service_dependency celix_dm_service_dependency_t;
@@ -63,8 +64,6 @@ typedef const struct celix_bundle * const_bundle_pt CELIX_DEPRECATED_ATTR;
 typedef struct celix_bundle bundle_t CELIX_DEPRECATED_ATTR;
 
 // will be deprecated in the future
-typedef struct bundleArchive *bundle_archive_pt;
-typedef struct bundleArchive bundle_archive_t;
 typedef struct service_factory *service_factory_pt;
 typedef struct serviceReference * service_reference_pt;
 typedef struct serviceRegistration service_registration_t;

libs/framework/src/bundle.c

@@ -17,8 +17,6 @@
  * under the License.
  */
 
-#include "bundle_revision_private.h"
-
 #include "celix_bundle_manifest.h"
 #include "celix_bundle_private.h"
 #include "celix_module.h"
@@ -27,20 +25,20 @@
 #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 <celix_constants.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);
 celix_status_t bundle_closeRevisions(const_bundle_pt bundle);
 
-celix_status_t celix_bundle_createFromArchive(celix_framework_t *framework, bundle_archive_pt archive, celix_bundle_t **bundleOut) {
+celix_status_t celix_bundle_createFromArchive(celix_framework_t *framework, celix_bundle_archive_t* archive, celix_bundle_t **bundleOut) {
     celix_status_t status = CELIX_SUCCESS;
     celix_bundle_t* bundle = calloc(1, sizeof(*bundle));
 
@@ -92,7 +90,7 @@ celix_status_t bundle_destroy(bundle_pt bundle) {
     return CELIX_SUCCESS;
 }
 
-celix_status_t bundle_getArchive(const_bundle_pt bundle, bundle_archive_pt *archive) {
+celix_status_t bundle_getArchive(const_bundle_pt bundle, celix_bundle_archive_t** archive) {
 	celix_status_t status = CELIX_SUCCESS;
 	if (bundle != NULL && *archive == NULL) {
 		*archive = bundle->archive;
@@ -201,22 +199,16 @@ celix_status_t bundle_addModule(bundle_pt bundle, celix_module_t* module) {
     return CELIX_SUCCESS;
 }
 
-celix_status_t bundle_isSystemBundle(const_bundle_pt bundle, bool *systemBundle) {
-	celix_status_t status;
-	long bundleId;
-	bundle_archive_pt archive = NULL;
-
-	status = bundle_getArchive(bundle, &archive);
-	if (status == CELIX_SUCCESS) {
-		status = bundleArchive_getId(archive, &bundleId);
-		if (status == CELIX_SUCCESS) {
-			*systemBundle = (bundleId == 0);
-		}
-	}
-
-	framework_logIfError(bundle->framework->logger, status, NULL, "Failed to check if bundle is the systembundle");
-
-	return status;
+celix_status_t bundle_isSystemBundle(const_bundle_pt bundle, bool* systemBundle) {
+    celix_bundle_archive_t* archive = celix_bundle_getArchive(bundle);
+    if (archive) {
+        long bundleId = celix_bundleArchive_getId(archive);
+        *systemBundle = (bundleId == CELIX_FRAMEWORK_BUNDLE_ID);
+    } else {
+        fw_log(bundle->framework->logger, CELIX_LOG_LEVEL_ERROR, "Failed to check if bundle is the systembundle");
+        return CELIX_BUNDLE_EXCEPTION;
+    }
+    return CELIX_SUCCESS;
 }
 
 celix_status_t bundle_getBundleId(const bundle_t *bundle, long *bndId) {
@@ -239,31 +231,6 @@ celix_status_t bundle_getFramework(const_bundle_pt bundle, framework_pt *framewo
     return CELIX_SUCCESS;
 }
 
-celix_status_t bundle_getBundleLocation(const_bundle_pt bundle, const char **location){
-
-	celix_status_t status;
-
-	bundle_archive_pt archive = NULL;
-
-	status = bundle_getArchive(bundle, &archive);
-	if (status != CELIX_SUCCESS){
-		printf("[ ERROR ]: Bundle - getBundleLocation (BundleArchive) \n");
-		return status;
-	}
-
-	status =  bundleArchive_getLocation(archive, location);
-	if (status != CELIX_SUCCESS){
-		printf("[ ERROR ]:  Bundle - getBundleLocation (BundleArchiveLocation) \n");
-		return status;
-	}
-
-	return CELIX_SUCCESS;
-}
-
-
-
-
-
 
 /**********************************************************************************************************************
  **********************************************************************************************************************
@@ -272,25 +239,25 @@ celix_status_t bundle_getBundleLocation(const_bundle_pt bundle, const char **loc
  **********************************************************************************************************************/
 
 long celix_bundle_getId(const bundle_t* bnd) {
-	long bndId = -1;
-	bundle_archive_pt archive = NULL;
-	bundle_getArchive((bundle_t*)bnd, &archive);
-	if (archive != NULL) {
-		bundleArchive_getId(archive, &bndId);
-	}
-
-	if (bndId < 0) {
-		framework_logIfError(celix_frameworkLogger_globalLogger(), CELIX_BUNDLE_EXCEPTION, NULL, "Failed to get bundle id");
-	}
-	return bndId;
+    long bndId = -1;
+    celix_bundle_archive_t* archive = celix_bundle_getArchive(bnd);
+    if (archive) {
+        bndId = celix_bundleArchive_getId(archive);
+    } else {
+        framework_logIfError(celix_frameworkLogger_globalLogger(),
+                             CELIX_BUNDLE_EXCEPTION,
+                             NULL,
+                             "Failed to get bundle id");
+    }
+    return bndId;
 }
 
 celix_bundle_state_e celix_bundle_getState(const celix_bundle_t *bnd) {
     return __atomic_load_n(&bnd->state, __ATOMIC_ACQUIRE);
 }
 
 static char* celix_bundle_getBundleOrPersistentStoreEntry(const celix_bundle_t* bnd, bool bundleEntry, const char* name) {
-    bundle_archive_pt archive = NULL;
+    celix_bundle_archive_t* archive = NULL;
     celix_status_t status = bundle_getArchive(bnd, &archive);
     if (status != CELIX_SUCCESS) {
         fw_logCode(bnd->framework->logger, CELIX_BUNDLE_EXCEPTION, status, "Failed to get bundle archive");
@@ -373,7 +340,11 @@ const char* celix_bundle_getDescription(const celix_bundle_t* bnd) {
 char* celix_bundle_getLocation(const celix_bundle_t *bnd) {
     char* result = NULL;
     if (bnd->archive != NULL) {
-        result = celix_bundleArchive_getLocation(bnd->archive);
+        const char* loc = celix_bundleArchive_getLocation(bnd->archive);
+        result = celix_utils_strdup(loc);
+        if (!result) {
+            fw_log(bnd->framework->logger, CELIX_BUNDLE_EXCEPTION, "Failed to allocate memory for bundle location");
+        }
     }
     return result;
 }
@@ -454,8 +425,8 @@ celix_array_list_t* celix_bundle_listServiceTrackers(const celix_bundle_t *bnd)
     return result;
 }
 
-bundle_archive_t* celix_bundle_getArchive(const celix_bundle_t *bundle) {
-    bundle_archive_t* archive = NULL;
-    bundle_getArchive(bundle, &archive);
+celix_bundle_archive_t* celix_bundle_getArchive(const celix_bundle_t *bundle) {
+    celix_bundle_archive_t* archive = NULL;
+    (void)bundle_getArchive(bundle, &archive);
     return archive;
 }