This is an automated email from the ASF dual-hosted git repository.

pengzheng pushed a commit to branch feature/556-osgi-uninstall-2
in repository https://gitbox.apache.org/repos/asf/celix.git

commit 415b8eb38ec98650c6f87a42fdd56d2ce2cc5fd6
Author: PengZheng <[email protected]>
AuthorDate: Sat Jun 3 15:13:31 2023 +0800

    Implement bundle update as bundle unload plus bundle install.
---
 .../src/CelixBundleCacheErrorInjectionTestSuite.cc |  2 +-
 .../gtest/src/CelixBundleCacheTestSuite.cc         | 16 ++++++-
 .../src/CelixBundleContextBundlesTestSuite.cc      |  3 +-
 libs/framework/include/celix_bundle_context.h      |  8 ++--
 libs/framework/include/celix_framework.h           | 16 +++----
 libs/framework/src/celix_bundle_cache.c            |  6 ++-
 libs/framework/src/celix_bundle_cache.h            | 11 +++--
 libs/framework/src/framework.c                     | 51 ++++++++++++----------
 8 files changed, 68 insertions(+), 45 deletions(-)

diff --git 
a/libs/framework/gtest/src/CelixBundleCacheErrorInjectionTestSuite.cc 
b/libs/framework/gtest/src/CelixBundleCacheErrorInjectionTestSuite.cc
index 38aa3b3c..7f0896c5 100644
--- a/libs/framework/gtest/src/CelixBundleCacheErrorInjectionTestSuite.cc
+++ b/libs/framework/gtest/src/CelixBundleCacheErrorInjectionTestSuite.cc
@@ -154,7 +154,7 @@ TEST_F(CelixBundleCacheErrorInjectionTestSuite, 
ArchiveDestroyErrorTest) {
     EXPECT_EQ(CELIX_SUCCESS, celix_bundleCache_createArchive(cache, 1, 
SIMPLE_TEST_BUNDLE1_LOCATION, &archive));
     
celix_ei_expect_celix_utils_deleteDirectory((void*)celix_bundleCache_destroyArchive,
 1, CELIX_FILE_IO_EXCEPTION);
     std::string storeRoot = 
celix_bundleArchive_getPersistentStoreRoot(archive);
-    EXPECT_EQ(CELIX_FILE_IO_EXCEPTION, celix_bundleCache_destroyArchive(cache, 
archive));
+    EXPECT_EQ(CELIX_FILE_IO_EXCEPTION, celix_bundleCache_destroyArchive(cache, 
archive, true));
     EXPECT_TRUE(celix_utils_directoryExists(storeRoot.c_str()));
     EXPECT_EQ(CELIX_SUCCESS, celix_bundleCache_destroy(cache));
 }
diff --git a/libs/framework/gtest/src/CelixBundleCacheTestSuite.cc 
b/libs/framework/gtest/src/CelixBundleCacheTestSuite.cc
index 945242db..c37ef142 100644
--- a/libs/framework/gtest/src/CelixBundleCacheTestSuite.cc
+++ b/libs/framework/gtest/src/CelixBundleCacheTestSuite.cc
@@ -58,12 +58,24 @@ TEST_F(CelixBundleCacheTestSuite, ArchiveCreateDestroyTest) 
{
     EXPECT_TRUE(celix_bundleCache_isBundleIdAlreadyUsed(fw.cache, 1));
     std::string loc = celix_bundleArchive_getPersistentStoreRoot(archive);
     EXPECT_TRUE(celix_utils_directoryExists(loc.c_str()));
-    EXPECT_EQ(CELIX_SUCCESS, celix_bundleCache_destroyArchive(fw.cache, 
archive));
+    EXPECT_EQ(CELIX_SUCCESS, celix_bundleCache_destroyArchive(fw.cache, 
archive, true));
     EXPECT_EQ(-1, celix_bundleCache_findBundleIdForLocation(fw.cache, 
SIMPLE_TEST_BUNDLE1_LOCATION));
     EXPECT_FALSE(celix_bundleCache_isBundleIdAlreadyUsed(fw.cache, 1));
     EXPECT_FALSE(celix_utils_directoryExists(loc.c_str()));
 }
 
+TEST_F(CelixBundleCacheTestSuite, NonPermanentDestroyTest) {
+    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);
+    EXPECT_TRUE(celix_utils_directoryExists(loc.c_str()));
+    EXPECT_EQ(CELIX_SUCCESS, celix_bundleCache_destroyArchive(fw.cache, 
archive, false));
+    EXPECT_EQ(1, celix_bundleCache_findBundleIdForLocation(fw.cache, 
SIMPLE_TEST_BUNDLE1_LOCATION));
+    EXPECT_TRUE(celix_bundleCache_isBundleIdAlreadyUsed(fw.cache, 1));
+    EXPECT_TRUE(celix_utils_directoryExists(loc.c_str()));
+}
+
 TEST_F(CelixBundleCacheTestSuite, SystemArchiveCreateDestroyTest) {
     bundle_archive_t* archive = nullptr;
     const char* archiveRoot = nullptr;
@@ -73,7 +85,7 @@ TEST_F(CelixBundleCacheTestSuite, 
SystemArchiveCreateDestroyTest) {
     EXPECT_EQ(CELIX_SUCCESS, bundleArchive_getArchiveRoot(archive, 
&archiveRoot));
     EXPECT_EQ(nullptr, archiveRoot);
     EXPECT_EQ(nullptr, celix_bundleArchive_getLocation(archive));
-    EXPECT_EQ(CELIX_SUCCESS, celix_bundleCache_destroyArchive(fw.cache, 
archive));
+    EXPECT_EQ(CELIX_SUCCESS, celix_bundleCache_destroyArchive(fw.cache, 
archive, true));
 }
 
 TEST_F(CelixBundleCacheTestSuite, CreateBundleArchivesCacheTest) {
diff --git a/libs/framework/gtest/src/CelixBundleContextBundlesTestSuite.cc 
b/libs/framework/gtest/src/CelixBundleContextBundlesTestSuite.cc
index 0d222bb7..c3b3df93 100644
--- a/libs/framework/gtest/src/CelixBundleContextBundlesTestSuite.cc
+++ b/libs/framework/gtest/src/CelixBundleContextBundlesTestSuite.cc
@@ -305,9 +305,10 @@ TEST_F(CelixBundleContextBundlesTestSuite, 
UpdateBundlesTest) {
     ASSERT_TRUE(bndId2 >= 0L);
     ASSERT_FALSE(celix_bundleContext_updateBundle(ctx, bndId1, TEST_BND2_LOC));
     ASSERT_TRUE(celix_bundleContext_isBundleInstalled(ctx, bndId1));
+
+    // remove it from cache before updating
     ASSERT_EQ(bndId2, celix_bundleContext_installBundle(ctx, TEST_BND2_LOC, 
false));
     ASSERT_TRUE(celix_bundleContext_uninstallBundle(ctx, bndId2));
-
     auto sn1 = celix_bundleContext_getBundleSymbolicName(ctx, bndId1);
     ASSERT_TRUE(celix_bundleContext_updateBundle(ctx, bndId1, TEST_BND2_LOC));
     auto sn2 = celix_bundleContext_getBundleSymbolicName(ctx, bndId1);
diff --git a/libs/framework/include/celix_bundle_context.h 
b/libs/framework/include/celix_bundle_context.h
index 3d3d71e1..40430d28 100644
--- a/libs/framework/include/celix_bundle_context.h
+++ b/libs/framework/include/celix_bundle_context.h
@@ -955,8 +955,8 @@ CELIX_FRAMEWORK_EXPORT bool 
celix_bundleContext_startBundle(celix_bundle_context
  * @brief Update the bundle with the provided bundle id.
  *
  * This will do the following:
- *  - uninstall the bundle with the specified bundle id;
- *  - reinstall the bundle from the specified location with the specified 
bundle id;
+ *  - unload the bundle with the specified bundle id;
+ *  - reload the bundle from the specified location with the specified bundle 
id;
  *  - start the bundle, if it was previously active.
  *
  * Will silently ignore bundle ids < 0.
@@ -971,8 +971,8 @@ CELIX_FRAMEWORK_EXPORT bool 
celix_bundleContext_startBundle(celix_bundle_context
  *
  * @param ctx The bundle context
  * @param bndId The bundle id to update.
- * @param updatedBundleUrl The optional updated bundle url to the bundle zip 
file. If NULL, the existing bundle url
- *                         from the bundle cache will be used.
+ * @param updatedBundleUrl The optional updated bundle url to the bundle zip 
file.
+ * If NULL, the existing bundle url from the bundle cache will be used, and 
the cache will only be updated if the zip file is newer.
  * @return true if the bundle is found & correctly started. False if not.
  */
 CELIX_FRAMEWORK_EXPORT bool 
celix_bundleContext_updateBundle(celix_bundle_context_t *ctx, long bndId, const 
char* updatedBundleUrl);
diff --git a/libs/framework/include/celix_framework.h 
b/libs/framework/include/celix_framework.h
index ad833ce5..16da33c4 100644
--- a/libs/framework/include/celix_framework.h
+++ b/libs/framework/include/celix_framework.h
@@ -186,19 +186,19 @@ CELIX_FRAMEWORK_EXPORT bool 
celix_framework_unloadBundle(celix_framework_t *fw,
  * @brief Update the bundle with the provided bundle id.
  *
  * This will do the following:
- *  - uninstall the bundle with the specified bundle id;
- *  - reinstall the bundle from the specified location with the specified 
bundle id;
+ *  - unload the bundle with the specified bundle id;
+ *  - reload the bundle from the specified location with the specified bundle 
id;
  *  - start the bundle, if it was previously active.
  *
  *  Will silently ignore bundle ids < 0.
  *
  *  Note if specified bundle location already exists in the bundle cache but 
with a different bundle id, the bundle
- *  will NOT be reinstalled, and the update is cancelled.
+ *  will NOT be reloaded, and the update is cancelled.
  *
  * @param [in] fw The Celix framework
  * @param [in] bndId the bundle id to update.
  * @param [in] updatedBundleUrl The optional updated bundle url to the bundle 
zip file.
- * If NULL, the existing bundle url from the bundle cache will be used.
+ * If NULL, the existing bundle url from the bundle cache will be used, and 
the cache will only be updated if the zip file is newer.
  * @return true if the bundle is correctly updated. False if not.
  */
 CELIX_FRAMEWORK_EXPORT bool celix_framework_updateBundle(celix_framework_t 
*fw, long bndId, const char* updatedBundleUrl);
@@ -240,8 +240,8 @@ CELIX_FRAMEWORK_EXPORT long 
celix_framework_installBundleAsync(celix_framework_t
  * @brief Update the bundle with the provided bundle id async.
  *
  * This will do the following:
- *  - uninstall the bundle with the specified bundle id;
- *  - reinstall the bundle from the specified location with the specified 
bundle id;
+ *  - unload the bundle with the specified bundle id;
+ *  - reload the bundle from the specified location with the specified bundle 
id;
  *  - start the bundle, if it was previously active.
  *
  *  Will silently ignore bundle ids < 0.
@@ -251,8 +251,8 @@ CELIX_FRAMEWORK_EXPORT long 
celix_framework_installBundleAsync(celix_framework_t
  *
  * @param [in] fw The Celix framework
  * @param [in] bndId the bundle id to update.
- * @param [in] updatedBundleUrl The optional updated bundle url to the bundle 
zip file. If NULL, the existing bundle url
- *                         from the bundle cache will be used.
+ * @param [in] updatedBundleUrl The optional updated bundle url to the bundle 
zip file.
+ * If NULL, the existing bundle url from the bundle cache will be used, and 
the cache will only be updated if the zip file is newer.
  */
 CELIX_FRAMEWORK_EXPORT void 
celix_framework_updateBundleAsync(celix_framework_t *fw, long bndId, const 
char* updatedBundleUrl);
 
diff --git a/libs/framework/src/celix_bundle_cache.c 
b/libs/framework/src/celix_bundle_cache.c
index aa0bbce8..94a5933d 100644
--- a/libs/framework/src/celix_bundle_cache.c
+++ b/libs/framework/src/celix_bundle_cache.c
@@ -230,13 +230,15 @@ celix_status_t 
celix_bundleCache_createSystemArchive(celix_framework_t* fw, bund
     return celix_bundleCache_createArchive(fw->cache, 
CELIX_FRAMEWORK_BUNDLE_ID, NULL, archive);
 }
 
-celix_status_t celix_bundleCache_destroyArchive(celix_bundle_cache_t* cache, 
bundle_archive_pt archive) {
+celix_status_t celix_bundleCache_destroyArchive(celix_bundle_cache_t* cache, 
bundle_archive_pt archive, bool permanent) {
     celix_status_t status = CELIX_SUCCESS;
     const char* loc = NULL;
     celixThreadMutex_lock(&cache->mutex);
     (void) bundleArchive_getLocation(archive, &loc);
     (void) celix_stringHashMap_remove(cache->locationToBundleIdLookupMap, loc);
-    status = bundleArchive_closeAndDelete(archive);
+    if (permanent) {
+        status = bundleArchive_closeAndDelete(archive);
+    }
     celixThreadMutex_unlock(&cache->mutex);
     (void) bundleArchive_destroy(archive);
     return status;
diff --git a/libs/framework/src/celix_bundle_cache.h 
b/libs/framework/src/celix_bundle_cache.h
index 14f5ea86..fb093705 100644
--- a/libs/framework/src/celix_bundle_cache.h
+++ b/libs/framework/src/celix_bundle_cache.h
@@ -20,12 +20,14 @@
 #ifndef CELIX_BUNDLE_CACHE_H_
 #define CELIX_BUNDLE_CACHE_H_
 
+#include <stdbool.h>
 
-#include "bundle_archive.h"
-#include "celix_framework.h"
 #include "celix_array_list.h"
+#include "celix_framework.h"
 #include "celix_long_hash_map.h"
 
+#include "bundle_archive.h"
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -89,15 +91,16 @@ celix_status_t 
celix_bundleCache_createSystemArchive(celix_framework_t* fw, bund
 
 /**
  * @brief Destroy the archive from the cache.
- * It releases all resources allocated in celix_bundleCache_createArchive and 
deletes the archive directory.
+ * It releases all resources allocated in celix_bundleCache_createArchive and 
deletes the archive directory if requested.
  * @param [in] cache The bundle cache to destroy archive from.
  * @param [in] archive The archive to destroy.
+ * @param [in] permanent Whether the archive directory should be deleted or 
not.
  * @return Status code indication failure or success:
  *      - CELIX_SUCCESS when no errors are encountered.
  *      - CELIX_FILE_IO_EXCEPTION when root of the archive is not a directory.
  *      - errno when the directory cannot be deleted for other reasons, check 
error codes of fts_open/fts_read/remove.
  */
-celix_status_t celix_bundleCache_destroyArchive(celix_bundle_cache_t* cache, 
bundle_archive_pt archive);
+celix_status_t celix_bundleCache_destroyArchive(celix_bundle_cache_t* cache, 
bundle_archive_pt archive, bool permanent);
 
 /**
  * @brief Deletes the entire bundle cache.
diff --git a/libs/framework/src/framework.c b/libs/framework/src/framework.c
index e730133c..0c3983eb 100644
--- a/libs/framework/src/framework.c
+++ b/libs/framework/src/framework.c
@@ -17,33 +17,35 @@
  * under the License.
  */
 
-#include <stdlib.h>
+#include <assert.h>
+#include <celix_log_utils.h>
+#include <stdbool.h>
 #include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
-#include <stdbool.h>
 #include <uuid/uuid.h>
-#include <assert.h>
-#include <celix_log_utils.h>
 
+#include "celix_build_assert.h"
+#include "celix_bundle_context.h"
+#include "celix_constants.h"
+#include "celix_convert_utils.h"
 #include "celix_dependency_manager.h"
+#include "celix_file_utils.h"
+#include "celix_framework_utils_private.h"
+#include "celix_libloader.h"
+#include "celix_log_constants.h"
+#include "celix_module_private.h"
+
+#include "bundle_archive_private.h"
+#include "bundle_context_private.h"
+#include "bundle_private.h"
 #include "framework_private.h"
-#include "celix_constants.h"
-#include "resolver.h"
-#include "utils.h"
 #include "linked_list_iterator.h"
+#include "resolver.h"
 #include "service_reference_private.h"
 #include "service_registration_private.h"
-#include "bundle_private.h"
-#include "celix_bundle_context.h"
-#include "bundle_context_private.h"
-#include "celix_libloader.h"
-#include "celix_log_constants.h"
-#include "celix_framework_utils_private.h"
-#include "bundle_archive_private.h"
-#include "celix_module_private.h"
-#include "celix_convert_utils.h"
-#include "celix_build_assert.h"
+#include "utils.h"
 
 struct celix_bundle_activator {
     void * userData;
@@ -1965,11 +1967,7 @@ celix_status_t 
celix_framework_uninstallBundleEntry(celix_framework_t* framework
         celix_framework_waitForEmptyEventQueue(framework); //to ensure that 
the uninstall event is triggered and handled
         (void)bundle_closeModules(bnd);
         (void)bundle_destroy(bnd);
-        if(permanent) {
-            (void)celix_bundleCache_destroyArchive(framework->cache, archive);
-        } else {
-            (void)bundleArchive_destroy(archive);
-        }
+        (void)celix_bundleCache_destroyArchive(framework->cache, archive, 
permanent);
     }
     celixThreadMutex_unlock(&framework->installLock);
     framework_logIfError(framework->logger, status, "", "Cannot uninstall 
bundle");
@@ -2291,6 +2289,7 @@ celix_status_t 
celix_framework_updateBundleEntry(celix_framework_t* framework,
     celix_status_t status = CELIX_SUCCESS;
     long bundleId = bndEntry->bndId;
     const char* errMsg;
+    char *bndRoot = NULL;
     fw_log(framework->logger, CELIX_LOG_LEVEL_DEBUG, "Updating bundle %s", 
celix_bundle_getSymbolicName(bndEntry->bnd));
     celix_bundle_state_e bndState = celix_bundle_getState(bndEntry->bnd);
     char *location = celix_bundle_getLocation(bndEntry->bnd);
@@ -2308,14 +2307,19 @@ celix_status_t 
celix_framework_updateBundleEntry(celix_framework_t* framework,
                 status = CELIX_ILLEGAL_STATE;
                 break;
             }
+            bndRoot = celix_bundle_getEntry(bndEntry->bnd, NULL);
         }
-        status = celix_framework_uninstallBundleEntry(framework, bndEntry, 
true);
+        status = celix_framework_uninstallBundleEntry(framework, bndEntry, 
false);
         if (status != CELIX_SUCCESS) {
             errMsg = "uninstall failure";
             celixThreadMutex_unlock(&framework->installLock);
             break;
         }
         // bndEntry is now invalid
+        if (bndRoot) {
+            // the bundle is updated with a new location, so the old bundle 
root can be removed
+            celix_utils_deleteDirectory(bndRoot, NULL);
+        }
         status = celix_framework_installBundleInternal(framework, 
updatedBundleUrl, &bundleId);
         if (status != CELIX_SUCCESS) {
             errMsg = "reinstall failure";
@@ -2338,6 +2342,7 @@ celix_status_t 
celix_framework_updateBundleEntry(celix_framework_t* framework,
         }
     } while(0);
     framework_logIfError(framework->logger, status, errMsg, "Could not update 
bundle from %s", updatedBundleUrl);
+    free(bndRoot);
     free(location);
     return status;
 }

Reply via email to