pnoltes commented on code in PR #476:
URL: https://github.com/apache/celix/pull/476#discussion_r1119903808


##########
libs/framework/src/celix_bundle_cache.c:
##########
@@ -71,124 +137,223 @@ celix_status_t 
celix_bundleCache_create(celix_framework_t* fw, celix_bundle_cach
         }
 
         asprintf(&cache->cacheDir, "/tmp/celix-cache-%s-%s", pg, 
celix_framework_getUUID(fw));
-        cache->deleteOnDestroy = true;
     } else {
+        const char* cacheDir = celix_bundleCache_cacheDirPath(fw);
         cache->cacheDir = celix_utils_strdup(cacheDir);
-        cache->deleteOnDestroy = false;
+    }
+
+    if (cache->deleteOnCreate) {
+        status = celix_bundleCache_deleteCacheDir(cache);
+        if (status != CELIX_SUCCESS) {
+            celix_bundleCache_destroy(cache);
+            return status;
+        }
+    }
+
+    const char* errorStr;
+    status = celix_utils_createDirectory(cache->cacheDir, false, &errorStr);
+    if (status != CELIX_SUCCESS) {
+        fw_logCode(fw->logger, CELIX_LOG_LEVEL_ERROR, status, "Cannot create 
bundle cache directory %s, error %s", cache->cacheDir, errorStr);
+        celix_bundleCache_destroy(cache);
+        return status;
     }
 
     *out = cache;
-       return CELIX_SUCCESS;
+       return status;
 }
 
 celix_status_t celix_bundleCache_destroy(celix_bundle_cache_t* cache) {
     celix_status_t status = CELIX_SUCCESS;
        if (cache->deleteOnDestroy) {
-        status = celix_bundleCache_delete(cache);
+        status = celix_bundleCache_deleteCacheDir(cache);
        }
        free(cache->cacheDir);
+    celix_stringHashMap_destroy(cache->locationToBundleIdLookupMap);
+    celixThreadMutex_destroy(&cache->mutex);
        free(cache);
        return status;
 }
 
-celix_status_t celix_bundleCache_delete(celix_bundle_cache_t* cache) {
+celix_status_t celix_bundleCache_deleteCacheDir(celix_bundle_cache_t* cache) {
     const char* err = NULL;
+    celixThreadMutex_lock(&cache->mutex);
     celix_status_t status = celix_utils_deleteDirectory(cache->cacheDir, &err);
-    if (err != NULL) {
-        FW_LOG(CELIX_LOG_LEVEL_ERROR, "Cannot delete cache dir at %s: %s", 
cache->cacheDir, err);
+    celixThreadMutex_unlock(&cache->mutex);
+    if (status != CELIX_SUCCESS) {
+        fw_logCode(cache->fw->logger, CELIX_LOG_LEVEL_ERROR, status, "Cannot 
delete bundle cache directory %s: %s", cache->cacheDir, err);
     }
     return status;
 }
 
-celix_status_t celix_bundleCache_getArchives(celix_bundle_cache_t* cache, 
array_list_pt *archives) {
+celix_status_t celix_bundleCache_createArchive(celix_framework_t* fw, long id, 
const char *location, bundle_archive_t** archiveOut) {
        celix_status_t status = CELIX_SUCCESS;
+    bundle_archive_t* archive = NULL;
+
+    char archiveRootBuffer[CELIX_DEFAULT_STRING_CREATE_BUFFER_SIZE];
+    char *archiveRoot = celix_utils_writeOrCreateString(archiveRootBuffer, 
sizeof(archiveRootBuffer), CELIX_BUNDLE_ARCHIVE_ROOT_FORMAT, 
fw->cache->cacheDir, id);
+    if (archiveRoot) {
+        celixThreadMutex_lock(&fw->cache->mutex);
+               status = bundleArchive_create(fw, archiveRoot, id, location, 
&archive);
+        celixThreadMutex_unlock(&fw->cache->mutex);
+        if (status != CELIX_SUCCESS) {
+            celix_utils_freeStringIfNeeded(archiveRootBuffer, archiveRoot);
+            return status;
+        }
+       } else {
+        status = CELIX_ENOMEM;
+        fw_logCode(fw->logger, CELIX_LOG_LEVEL_ERROR, status, "Failed to 
create archive. Out of memory.");
+        return status;
+    }
 
-       DIR *dir;
-       struct stat st;
+    //add bundle id and location to lookup maps
+    celixThreadMutex_lock(&fw->cache->mutex);
+    celix_stringHashMap_put(fw->cache->locationToBundleIdLookupMap, location, 
(void*)id);
+    celixThreadMutex_unlock(&fw->cache->mutex);
 
-       dir = opendir(cache->cacheDir);
+    *archiveOut = archive;
+       return status;
+}
 
-       if (dir == NULL && errno == ENOENT) {
-               if(mkdir(cache->cacheDir, S_IRWXU) == 0 ){
-                       dir = opendir(cache->cacheDir);
-               }
-       }
+celix_status_t celix_bundleCache_createSystemArchive(celix_framework_t* fw, 
bundle_archive_pt* archive) {
+    return celix_bundleCache_createArchive(fw, CELIX_FRAMEWORK_BUNDLE_ID, 
NULL, archive);
+}
 
-       if (dir != NULL) {
-               array_list_pt list = NULL;
-               arrayList_create(&list);
-
-               struct dirent* dent = NULL;
-
-               errno = 0;
-               dent = readdir(dir);
-               while (errno == 0 && dent != NULL) {
-                       char archiveRoot[512];
-
-                       snprintf(archiveRoot, sizeof(archiveRoot), "%s/%s", 
cache->cacheDir, dent->d_name);
-
-                       if (stat (archiveRoot, &st) == 0) {
-                               if (S_ISDIR (st.st_mode)
-                                               && (strcmp((dent->d_name), ".") 
!= 0)
-                                               && (strcmp((dent->d_name), 
"..") != 0)
-                                               && (strncmp(dent->d_name, 
"bundle", 6) == 0)
-                                               && (strcmp(dent->d_name, 
"bundle0") != 0)) {
-
-                                       bundle_archive_pt archive = NULL;
-                                       status = 
bundleArchive_recreate(cache->fw, archiveRoot, &archive);
-                                       if (status == CELIX_SUCCESS) {
-                                               arrayList_add(list, archive);
-                                       }
-                               }
-                       }
-
-                       errno = 0;
-                       dent = readdir(dir);
-               }
-
-               if (errno != 0) {
-                       fw_log(celix_frameworkLogger_globalLogger(), 
CELIX_LOG_LEVEL_ERROR, "Error reading dir");
-                       status = CELIX_FILE_IO_EXCEPTION;
-               } else {
-                       status = CELIX_SUCCESS;
-               }
-
-               closedir(dir);
-
-               if (status == CELIX_SUCCESS) {
-                       *archives = list;
-               }
-               else{
-                       int idx = 0;
-                       for(;idx<arrayList_size(list);idx++){
-                               
bundleArchive_destroy((bundle_archive_pt)arrayList_get(list,idx));
-                       }
-                       arrayList_destroy(list);
-                       *archives = NULL;
-               }
+/**
+ * Update location->bundle id lookup map.
+ * Assumes that bundle cache dir are not removed, so only adding not removing 
entries.
+ */
+static void celix_bundleCache_updateIdForLocationLookupMap(celix_framework_t* 
fw) {
+    celixThreadMutex_lock(&fw->cache->mutex);
+    DIR* dir = opendir(fw->cache->cacheDir);
+    if (dir == NULL) {
+        fw_logCode(fw->logger, CELIX_LOG_LEVEL_ERROR, CELIX_BUNDLE_EXCEPTION, 
"Cannot open bundle cache directory %s", fw->cache->cacheDir);
+        return;
+    }
+    char archiveRootBuffer[CELIX_DEFAULT_STRING_CREATE_BUFFER_SIZE];
+    struct dirent* dent = NULL;
+    while ((dent = readdir(dir)) != NULL) {
+        if (strncmp(dent->d_name, "bundle", 6) != 0) {
+            continue;
+        }
+        char *bundleStateProperties = 
celix_utils_writeOrCreateString(archiveRootBuffer, sizeof(archiveRootBuffer),
+                                                                      
"%s/%s/%s", fw->cache->cacheDir, dent->d_name,
+                                                                      
CELIX_BUNDLE_ARCHIVE_STATE_PROPERTIES_FILE_NAME);
+        if (celix_utils_fileExists(bundleStateProperties)) {
+            celix_properties_t *props = 
celix_properties_load(bundleStateProperties);
+            const char *visitLoc = celix_properties_get(props, 
CELIX_BUNDLE_ARCHIVE_LOCATION_PROPERTY_NAME, NULL);
+            long bndId = celix_properties_getAsLong(props, 
CELIX_BUNDLE_ARCHIVE_BUNDLE_ID_PROPERTY_NAME, -1);
+            if (visitLoc != NULL && bndId >= 0) {
+                fw_log(fw->logger, CELIX_LOG_LEVEL_TRACE, "Adding location %s 
-> bnd id %li to lookup map",
+                       visitLoc, bndId);
+                
celix_stringHashMap_putLong(fw->cache->locationToBundleIdLookupMap, visitLoc, 
bndId);
+            }
+            celix_properties_destroy(props);
+        }
+    }
+    closedir(dir);
+    celixThreadMutex_unlock(&fw->cache->mutex);
+}
 
-       } else {
-               status = CELIX_FILE_IO_EXCEPTION;
-       }
+long celix_bundleCache_findBundleIdForLocation(celix_framework_t *fw, const 
char *location) {
+    long bndId = -1;
+    celixThreadMutex_lock(&fw->cache->mutex);
+    if (celix_stringHashMap_hasKey(fw->cache->locationToBundleIdLookupMap, 
location)) {
+        bndId = 
celix_stringHashMap_getLong(fw->cache->locationToBundleIdLookupMap, location, 
-1);
+    }
+    celixThreadMutex_unlock(&fw->cache->mutex);
+    if (bndId == -1) {
+        celix_bundleCache_updateIdForLocationLookupMap(fw);
+        celixThreadMutex_lock(&fw->cache->mutex);
+        bndId = 
celix_stringHashMap_getLong(fw->cache->locationToBundleIdLookupMap, location, 
-1);
+        celixThreadMutex_unlock(&fw->cache->mutex);
+    }
+    return bndId;
+}
 
-       framework_logIfError(celix_frameworkLogger_globalLogger(), status, 
NULL, "Failed to get bundle archives");
-       if (status != CELIX_SUCCESS) {
-               perror("\t");
-       }
+bool celix_bundleCache_isBundleIdAlreadyUsed(celix_framework_t *fw, long 
bndId) {
+    bool found = false;
+    celixThreadMutex_lock(&fw->cache->mutex);
+    CELIX_STRING_HASH_MAP_ITERATE(fw->cache->locationToBundleIdLookupMap, 
iter) {
+        if (iter.value.longValue == bndId) {
+            found = true;
+            break;
+        }
+    }
+    celixThreadMutex_unlock(&fw->cache->mutex);
+    return found;
+}
 
-       return status;
+
+static celix_status_t 
celix_bundleCache_createBundleArchivesForSpaceSeparatedList(celix_framework_t 
*fw, long* bndId, const char* list, bool logProgress) {
+    celix_status_t status = CELIX_SUCCESS;
+    char delims[] = " ";
+    char *savePtr = NULL;
+    char zipFileListBuffer[CELIX_DEFAULT_STRING_CREATE_BUFFER_SIZE];
+    char* zipFileList = celix_utils_writeOrCreateString(zipFileListBuffer, 
sizeof(zipFileListBuffer), "%s", list);
+    if (zipFileList) {
+        char *location = strtok_r(zipFileList, delims, &savePtr);
+        while (location != NULL) {
+            bundle_archive_t* archive = NULL;
+            status = celix_bundleCache_createArchive(fw, (*bndId)++, location, 
&archive);
+            if (status != CELIX_SUCCESS) {
+                fw_logCode(fw->logger, CELIX_LOG_LEVEL_ERROR, 
CELIX_BUNDLE_EXCEPTION, "Cannot create bundle archive for %s", location);
+                break;
+            } else {
+                celix_log_level_e lvl = logProgress ? CELIX_LOG_LEVEL_INFO : 
CELIX_LOG_LEVEL_DEBUG;
+                fw_log(fw->logger, lvl, "Created bundle cache '%s' for bundle 
archive %s (bndId=%li).",
+                       celix_bundleArchive_getCurrentRevisionRoot(archive),
+                       celix_bundleArchive_getSymbolicName(archive), 
celix_bundleArchive_getId(archive));
+                bundleArchive_destroy(archive);
+            }
+            location = strtok_r(NULL, delims, &savePtr);
+        }
+    } else {
+        status = CELIX_ENOMEM;
+        fw_logCode(fw->logger, CELIX_LOG_LEVEL_ERROR, status, "Failed to 
create zip file list.");
+    }
+    celix_utils_freeStringIfNeeded(zipFileListBuffer, zipFileList);
+    return status;
 }
 
-celix_status_t celix_bundleCache_createArchive(celix_framework_t* fw, long id, 
const char * location, const char *inputFile, bundle_archive_pt *archive) {
-       celix_status_t status = CELIX_SUCCESS;
-       char archiveRoot[512];
+//using tmp cache and that bundle cache is not deleted during bundle cache 
destroy.
+celix_status_t celix_bundleCache_createBundleArchivesCache(celix_framework_t 
*fw, bool logProgress) {
+    celix_status_t status = CELIX_SUCCESS;
 
-       if (fw && location) {
-               snprintf(archiveRoot, sizeof(archiveRoot), "%s/bundle%ld",  
fw->cache->cacheDir, id);
-               status = bundleArchive_create(fw, archiveRoot, id, location, 
inputFile, archive);
-       }
+    const char* const cosgiKeys[] = 
{"cosgi.auto.start.0","cosgi.auto.start.1","cosgi.auto.start.2","cosgi.auto.start.3","cosgi.auto.start.4","cosgi.auto.start.5","cosgi.auto.start.6",
 NULL};
+    const char* const celixKeys[] = {CELIX_AUTO_START_0, CELIX_AUTO_START_1, 
CELIX_AUTO_START_2, CELIX_AUTO_START_3, CELIX_AUTO_START_4, CELIX_AUTO_START_5, 
CELIX_AUTO_START_6, NULL};
+    CELIX_BUILD_ASSERT(sizeof(*cosgiKeys) == sizeof(*celixKeys));
+    long bndId = CELIX_FRAMEWORK_BUNDLE_ID+1; //note cleaning cache, so 
starting bundle id at 1
 
-       framework_logIfError(fw->logger, status, NULL, "Failed to create 
archive");
+    const char* errorStr = NULL;
+    status = celix_utils_deleteDirectory(fw->cache->cacheDir, &errorStr);

Review Comment:
   Good catch and indeed something we should fix. Seeing that you created a 
issue for this, I assume that we will do this in a separate PR.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@celix.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to