[grilo] core: Refactor the way of loading plugins

[Juan A. Suarez Romero <jasuarez src gnome org>]

commit 961c9e20a0ffed6a8616a0bba393855e7829d241
Author: Juan A. Suarez Romero <jasuarez igalia com>
Date:   Wed Jul 27 15:21:34 2011 +0000

    core: Refactor the way of loading plugins
    
    Plugins' XML information files are loaded before the plugins themselves, so the
    list of plugins and their identifiers are available to the user, who can just
    load a specific subset of them.
    
    As those XML files are just loaded once, if user unloads and reload again some
    or all plugins, there is no need to re-scan all the directories, as the plugins
    locations are already loaded and known.
    
    Signed-off-by: Juan A. Suarez Romero <jasuarez igalia com>

 src/grl-plugin-registry.c |  382 +++++++++++++++++++++++++--------------------
 src/grl-plugin-registry.h |    2 +-
 2 files changed, 215 insertions(+), 169 deletions(-)
---
diff --git a/src/grl-plugin-registry.c b/src/grl-plugin-registry.c
index 47ff561..cf8dfd1 100644
--- a/src/grl-plugin-registry.c
+++ b/src/grl-plugin-registry.c
@@ -77,16 +77,32 @@ struct _GrlPluginRegistryPrivate {
   GParamSpecPool *system_keys;
   GHashTable *ranks;
   GSList *plugins_dir;
+  gboolean all_plugin_info_loaded;
   struct KeyIDHandler key_id_handler;
 };
 
 static void grl_plugin_registry_setup_ranks (GrlPluginRegistry *registry);
-static void grl_plugin_registry_load_plugin_infos (GrlPluginRegistry *registry);
+
+static GList *grl_plugin_registry_load_plugin_info_directory (GrlPluginRegistry *registry,
+                                                              const gchar *path,
+                                                              GError **error);
+
+static GrlPluginInfo *grl_plugin_registry_load_plugin_info (GrlPluginRegistry *registry,
+                                                            const gchar *plugin_id,
+                                                            const gchar *file);
+
+static void grl_plugin_registry_load_plugin_info_all (GrlPluginRegistry *registry);
 
 static void key_id_handler_init (struct KeyIDHandler *handler);
-static GrlKeyID key_id_handler_get_key (struct KeyIDHandler *handler, const gchar *key_name);
-static const gchar *key_id_handler_get_name (struct KeyIDHandler *handler, GrlKeyID key);
-static GrlKeyID key_id_handler_add (struct KeyIDHandler *handler, GrlKeyID key, const gchar *key_name);
+
+static GrlKeyID key_id_handler_get_key (struct KeyIDHandler *handler,
+                                        const gchar *key_name);
+
+static const gchar *key_id_handler_get_name (struct KeyIDHandler *handler,
+                                             GrlKeyID key);
+
+static GrlKeyID key_id_handler_add (struct KeyIDHandler *handler,
+                                    GrlKeyID key, const gchar *key_name);
 
 /* ================ GrlPluginRegistry GObject ================ */
 
@@ -160,7 +176,6 @@ grl_plugin_registry_init (GrlPluginRegistry *registry)
   key_id_handler_init (&registry->priv->key_id_handler);
 
   grl_plugin_registry_setup_ranks (registry);
-  grl_plugin_registry_load_plugin_infos (registry);
 }
 
 /* ================ Utitilies ================ */
@@ -291,43 +306,133 @@ get_info_from_plugin_xml (const gchar *xml_path)
   return hash_table;
 }
 
-static void
-grl_plugin_registry_load_plugin_infos (GrlPluginRegistry *registry)
+static GrlPluginInfo *
+grl_plugin_registry_load_plugin_info (GrlPluginRegistry *registry,
+                                      const gchar* plugin_id,
+                                      const gchar *file)
 {
-  GDir *dir;
-  GError *error = NULL;
   GHashTable *info;
   GrlPluginInfo *plugin_info;
+  gchar *library_filename;
+  gchar *path;
+  gchar *plugin_name;
+
+  /* Load plugin information */
+  info = get_info_from_plugin_xml (file);
+
+  if (!info) {
+    GRL_WARNING ("Invalid information file for '%s' plugin",
+                 plugin_id);
+    return NULL;
+  }
+
+  /* Build plugin library filename */
+  plugin_name = g_hash_table_lookup (info, GRL_PLUGIN_INFO_MODULE);
+  if (!plugin_name) {
+    GRL_WARNING ("Information about '%s' plugin has no reference to module; skipping",
+                 plugin_id);
+    g_hash_table_unref (info);
+    return NULL;
+  }
+  plugin_name = g_strconcat (plugin_name, "." G_MODULE_SUFFIX, NULL);
+  path = g_path_get_dirname (file);
+  library_filename = g_build_filename (path, plugin_name, NULL);
+  g_free (plugin_name);
+  g_free (path);
+
+  /* Build Plugin Info */
+  plugin_info = g_new0 (GrlPluginInfo, 1);
+  plugin_info->id = g_strdup (plugin_id);
+  plugin_info->filename = library_filename;
+  plugin_info->optional_info = info;
+
+  /* Set rank */
+  set_plugin_rank (registry, plugin_info);
+
+  return plugin_info;
+}
+
+static GList *
+grl_plugin_registry_load_plugin_info_directory (GrlPluginRegistry *registry,
+                                                const gchar *path,
+                                                GError **error)
+{
+  GDir *dir;
+  GrlPluginInfo *plugin_info;
   const gchar *entry;
   gchar *file;
+  gchar *id;
   gchar *suffix;
+  GList *loaded_infos = NULL;
 
-  dir = g_dir_open (GRL_PLUGINS_DIR, 0, &error);
+  dir = g_dir_open (path, 0, NULL);
   if (!dir) {
-    GRL_WARNING ("Could not open plugins' info directory '%s': %s",
-                 GRL_PLUGINS_DIR,
-                 error->message);
-    g_error_free (error);
-    return;
+    GRL_WARNING ("Could not open plugin directory: '%s'", path);
+    g_set_error (error,
+                 GRL_CORE_ERROR,
+                 GRL_CORE_ERROR_LOAD_PLUGIN_FAILED,
+                 "Failed to open plugin directory '%s'", path);
+    return NULL;
   }
 
   while ((entry = g_dir_read_name (dir)) != NULL) {
     if ((suffix = g_strrstr (entry, "." GRL_PLUGIN_INFO_SUFFIX)) != NULL) {
-      file = g_build_filename (GRL_PLUGINS_DIR, entry, NULL);
-      info = get_info_from_plugin_xml (file);
-      g_free (file);
-      if (info) {
-        plugin_info = g_new0 (GrlPluginInfo, 1);
-        plugin_info->id = g_strndup (entry, suffix - entry);
-        plugin_info->optional_info = info;
-        g_hash_table_insert (registry->priv->plugin_infos,
-                             plugin_info->id,
-                             plugin_info);
+      file = g_build_filename (path, entry, NULL);
+      id = g_strndup (entry, suffix - entry);
+      /* Skip plugin info if it is already loaded */
+      if (g_hash_table_lookup (registry->priv->plugin_infos, id)) {
+        GRL_DEBUG ("Information about '%s' plugin already loaded; skipping",
+                   id);
+        g_free (id);
+        g_free (file);
+        continue;
       }
+      plugin_info = grl_plugin_registry_load_plugin_info (registry, id, file);
+      g_free (id);
+      g_free (file);
+
+      g_hash_table_insert (registry->priv->plugin_infos,
+                           plugin_info->id,
+                           plugin_info);
+      loaded_infos = g_list_append (loaded_infos, plugin_info);
     }
   }
 
   g_dir_close (dir);
+  return loaded_infos;
+}
+
+static void
+grl_plugin_registry_load_plugin_info_all (GrlPluginRegistry *registry)
+{
+  GSList *plugin_dir;
+  GList *loaded_plugins;
+
+  for (plugin_dir = registry->priv->plugins_dir;
+       plugin_dir;
+       plugin_dir = g_slist_next (plugin_dir)) {
+    loaded_plugins =
+      grl_plugin_registry_load_plugin_info_directory (registry,
+                                                      plugin_dir->data,
+                                                      NULL);
+    g_list_free (loaded_plugins);
+  }
+}
+
+static gboolean
+grl_plugin_registry_load_list (GrlPluginRegistry *registry,
+                               GList *plugin_info_list)
+{
+  GrlPluginInfo *pinfo;
+  gboolean loaded_one = FALSE;
+
+  while (plugin_info_list) {
+    pinfo = (GrlPluginInfo *) plugin_info_list->data;
+    loaded_one |= grl_plugin_registry_load (registry, pinfo->filename, NULL);
+    plugin_info_list = g_list_next (plugin_info_list);
+  }
+
+  return loaded_one;
 }
 
 static void
@@ -558,7 +663,7 @@ grl_plugin_registry_add_directory (GrlPluginRegistry *registry,
 /**
  * grl_plugin_registry_load:
  * @registry: the registry instance
- * @path: the path to the so file
+ * @library_filename: the path to the so file
  * @error: error return location or @NULL to ignore
  *
  * Loads a module from shared object file stored in @path
@@ -569,54 +674,57 @@ grl_plugin_registry_add_directory (GrlPluginRegistry *registry,
  */
 gboolean
 grl_plugin_registry_load (GrlPluginRegistry *registry,
-                          const gchar *path,
+                          const gchar *library_filename,
                           GError **error)
 {
   GModule *module;
   GrlPluginDescriptor *plugin;
   GrlPluginInfo *plugin_info;
   GList *plugin_configs;
+  gchar *dirname;
+  gchar *plugin_info_filename;
+  gchar *plugin_info_fullpathname;
 
   g_return_val_if_fail (GRL_IS_PLUGIN_REGISTRY (registry), FALSE);
 
-  module = g_module_open (path, G_MODULE_BIND_LAZY);
+  module = g_module_open (library_filename, G_MODULE_BIND_LAZY);
   if (!module) {
-    GRL_WARNING ("Failed to open module: '%s'", path);
+    GRL_WARNING ("Failed to open module: '%s'", library_filename);
     g_set_error (error,
                  GRL_CORE_ERROR,
                  GRL_CORE_ERROR_LOAD_PLUGIN_FAILED,
-                 "Failed to load plugin at '%s'", path);
+                 "Failed to load plugin at '%s'", library_filename);
     return FALSE;
   }
 
   if (!g_module_symbol (module, "GRL_PLUGIN_DESCRIPTOR", (gpointer) &plugin)) {
-    GRL_WARNING ("Did not find plugin descriptor: '%s'", path);
+    GRL_WARNING ("Did not find plugin descriptor: '%s'", library_filename);
     g_set_error (error,
                  GRL_CORE_ERROR,
                  GRL_CORE_ERROR_LOAD_PLUGIN_FAILED,
-                 "'%s' is not a valid plugin file", path);
+                 "'%s' is not a valid plugin file", library_filename);
     g_module_close (module);
     return FALSE;
   }
 
   if (!plugin->plugin_init ||
       !plugin->plugin_id) {
-    GRL_WARNING ("Plugin descriptor is not valid: '%s'", path);
+    GRL_WARNING ("Plugin descriptor is not valid: '%s'", library_filename);
     g_set_error (error,
                  GRL_CORE_ERROR,
                  GRL_CORE_ERROR_LOAD_PLUGIN_FAILED,
-                 "'%s' is not a valid plugin file", path);
+                 "'%s' is not a valid plugin file", library_filename);
     g_module_close (module);
     return FALSE;
   }
 
   /* Check if plugin is already loaded */
   if (g_hash_table_lookup (registry->priv->plugins, plugin->plugin_id)) {
-    GRL_WARNING ("Plugin is already loaded: '%s'", path);
+    GRL_WARNING ("Plugin is already loaded: '%s'", library_filename);
     g_set_error (error,
                  GRL_CORE_ERROR,
                  GRL_CORE_ERROR_LOAD_PLUGIN_FAILED,
-                 "'%s' is already loaded", path);
+                 "'%s' is already loaded", library_filename);
     g_module_close (module);
     return FALSE;
   }
@@ -624,63 +732,66 @@ grl_plugin_registry_load (GrlPluginRegistry *registry,
   plugin_info = g_hash_table_lookup (registry->priv->plugin_infos,
                                      plugin->plugin_id);
 
+  /* This happens when the user invokes directly this function: plugin
+     information has not been loaded yet */
   if (!plugin_info) {
-    plugin_info = g_new0 (GrlPluginInfo, 1);
-    plugin_info->id = plugin->plugin_id;
-    g_hash_table_insert (registry->priv->plugin_infos,
-                         plugin_info->id,
-                         plugin_info);
+    plugin_info_filename = g_strconcat (plugin->plugin_id,
+                                        "." GRL_PLUGIN_INFO_SUFFIX,
+                                        NULL);
+    dirname = g_path_get_dirname (library_filename);
+    plugin_info_fullpathname = g_build_filename (dirname,
+                                                 plugin_info_filename,
+                                                 NULL);
+    plugin_info = grl_plugin_registry_load_plugin_info (registry,
+                                                        plugin->plugin_id,
+                                                        plugin_info_filename);
+    if (!plugin_info) {
+      GRL_WARNING ("Plugin '%s' does not have XML information file '%s'",
+                   plugin->plugin_id,
+                   plugin_info_fullpathname);
+      /* Create a default one */
+      plugin_info = g_new0 (GrlPluginInfo, 1);
+      plugin_info->id = g_strdup (plugin->plugin_id);
+      plugin_info->filename = g_strdup (library_filename);
+      g_hash_table_insert (registry->priv->plugin_infos,
+                           plugin_info->id,
+                           plugin_info);
+      plugin_info->optional_info =
+        g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free);
+      g_hash_table_insert (plugin_info->optional_info,
+                           g_strdup (GRL_PLUGIN_INFO_MODULE),
+                           g_path_get_basename (plugin_info->filename));
+
+      set_plugin_rank (registry, plugin_info);
+    }
+    g_free (plugin_info_filename);
+    g_free (dirname);
+    g_free (plugin_info_fullpathname);
   }
 
-  set_plugin_rank (registry, plugin_info);
   g_hash_table_insert (registry->priv->plugins,
                        (gpointer) plugin->plugin_id, plugin);
 
-  /* Plugin info can have a valid filename if the plugin was loaded previously
-     (and unloaded afterwards). Note that we do not free filename when plugin is
-     unloaded because this way, if user tries to load the plugin with
-     load_by_id(), it will not need to search for the plugin throughout the list
-     of plugin directories: filename will be already pointing to the right
-     filename */
-  if (plugin_info->filename) {
-    g_free (plugin_info->filename);
-  }
-  plugin_info->filename = g_strdup (path);
-
   plugin_configs = g_hash_table_lookup (registry->priv->configs,
-					plugin->plugin_id);
+                                        plugin->plugin_id);
 
   if (!plugin->plugin_init (registry, plugin_info, plugin_configs)) {
-    g_free (plugin_info->filename);
-    plugin_info->filename = NULL;
     g_hash_table_remove (registry->priv->plugins, plugin->plugin_id);
-    GRL_INFO ("Failed to initialize plugin: '%s'", path);
+    GRL_INFO ("Failed to initialize plugin: '%s'", library_filename);
     g_set_error (error,
                  GRL_CORE_ERROR,
                  GRL_CORE_ERROR_LOAD_PLUGIN_FAILED,
-                 "Failed to initialize plugin at '%s'", path);
+                 "Failed to initialize plugin at '%s'", library_filename);
     g_module_close (module);
     return FALSE;
   }
 
-  /* Insert module name as part of plugin information */
-  if (!plugin_info->optional_info) {
-    plugin_info->optional_info =
-      g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free);
-  }
-  if (!g_hash_table_lookup (plugin_info->optional_info,
-                            GRL_PLUGIN_INFO_MODULE)) {
-    g_hash_table_insert (plugin_info->optional_info,
-                         g_strdup (GRL_PLUGIN_INFO_MODULE),
-                         g_path_get_basename (plugin_info->filename));
-  }
-
   /* Make plugin resident */
   g_module_make_resident (module);
 
   plugin->module = module;
 
-  GRL_DEBUG ("Loaded plugin '%s' from '%s'", plugin->plugin_id, path);
+  GRL_DEBUG ("Loaded plugin '%s' from '%s'", plugin->plugin_id, library_filename);
 
   return TRUE;
 }
@@ -703,39 +814,21 @@ grl_plugin_registry_load_directory (GrlPluginRegistry *registry,
                                     const gchar *path,
                                     GError **error)
 {
-  GDir *dir;
-  gchar *file;
-  const gchar *entry;
-  gboolean loaded_one = FALSE;
+  GList *plugin_infos;
 
   g_return_val_if_fail (GRL_IS_PLUGIN_REGISTRY (registry), FALSE);
 
-  dir = g_dir_open (path, 0, NULL);
-
-  if (!dir) {
-    GRL_WARNING ("Could not open plugin directory: '%s'", path);
-    g_set_error (error,
-                 GRL_CORE_ERROR,
-                 GRL_CORE_ERROR_LOAD_PLUGIN_FAILED,
-                 "Failed to open plugin directory '%s'", path);
-    return FALSE;
-  }
-
-  while ((entry = g_dir_read_name (dir)) != NULL) {
-    if (g_str_has_suffix (entry, "." G_MODULE_SUFFIX)) {
-      file = g_build_filename (path, entry, NULL);
-      if (grl_plugin_registry_load (registry, file, NULL)) {
-        loaded_one = TRUE;
-      }
-      g_free (file);
-    }
-  }
+  /* Load plugins information */
+  plugin_infos = grl_plugin_registry_load_plugin_info_directory (registry,
+                                                                 path,
+                                                                 error);
 
-  if (!loaded_one) {
+  /* Load the plugins */
+  if (!grl_plugin_registry_load_list (registry, plugin_infos)) {
     GRL_WARNING ("No plugins loaded from directory '%s'", path);
   }
+  g_list_free (plugin_infos);
 
-  g_dir_close (dir);
   return TRUE;
 }
 
@@ -758,24 +851,28 @@ grl_plugin_registry_load_directory (GrlPluginRegistry *registry,
 gboolean
 grl_plugin_registry_load_all (GrlPluginRegistry *registry, GError **error)
 {
-  GSList *plugin_dir;
-  gboolean loaded_one = FALSE;
+  GList *all_plugin_infos;
+  gboolean loaded_one;
 
   g_return_val_if_fail (GRL_IS_PLUGIN_REGISTRY (registry), TRUE);
 
-  for (plugin_dir = registry->priv->plugins_dir;
-       plugin_dir;
-       plugin_dir = g_slist_next (plugin_dir)) {
-    if (grl_plugin_registry_load_directory (registry, plugin_dir->data, NULL)) {
-      loaded_one = TRUE;
-    }
+  /* Preload all plugin infos */
+  if (!registry->priv->all_plugin_info_loaded) {
+    grl_plugin_registry_load_plugin_info_all (registry);
+    registry->priv->all_plugin_info_loaded = TRUE;
   }
 
+  /* Now load all plugins */
+  all_plugin_infos = g_hash_table_get_values (registry->priv->plugin_infos);
+  loaded_one = grl_plugin_registry_load_list (registry, all_plugin_infos);
+
+  g_list_free (all_plugin_infos);
+
   if (!loaded_one) {
     g_set_error (error,
                  GRL_CORE_ERROR,
                  GRL_CORE_ERROR_LOAD_PLUGIN_FAILED,
-                 "All configured plugin paths are invalid. "    \
+                 "All configured plugin paths are invalid. "   \
                  "Failed to load plugins.");
   }
 
@@ -803,17 +900,21 @@ grl_plugin_registry_load_by_id (GrlPluginRegistry *registry,
                                 const gchar *plugin_id,
                                 GError **error)
 {
-  GSList *plugin_dir;
   GrlPluginInfo *plugin_info;
-  const gchar *module_name;
-  gboolean result;
-  gchar *module_filename;
-  gchar *module_fullpathname;
 
   g_return_val_if_fail (GRL_IS_PLUGIN_REGISTRY (registry), FALSE);
 
-  /* Check if there is information preloaded */
+  /* Check if there is information loaded */
   plugin_info = g_hash_table_lookup (registry->priv->plugin_infos, plugin_id);
+    /* Maybe we need to load all plugins before */
+  if (!plugin_info &&
+      !registry->priv->all_plugin_info_loaded) {
+    grl_plugin_registry_load_plugin_info_all (registry);
+    registry->priv->all_plugin_info_loaded = TRUE;
+    /* Search again */
+    plugin_info = g_hash_table_lookup (registry->priv->plugin_infos, plugin_id);
+  }
+
   if (!plugin_info) {
     GRL_WARNING ("There is no information about a plugin with id '%s'", plugin_id);
     g_set_error (error,
@@ -823,62 +924,7 @@ grl_plugin_registry_load_by_id (GrlPluginRegistry *registry,
     return FALSE;
   }
 
-  /* Check if we know the module full pathname */
-  if (plugin_info->filename) {
-    return grl_plugin_registry_load (registry, plugin_info->filename, error);
-  }
-
-  /* Check the module name */
-  if (plugin_info->optional_info) {
-    module_name = g_hash_table_lookup (plugin_info->optional_info,
-                                       GRL_PLUGIN_INFO_MODULE);
-  }
-
-  if (!module_name) {
-    GRL_WARNING ("Unknown module file for plugin with id '%s'", plugin_id);
-    g_set_error (error,
-                 GRL_CORE_ERROR,
-                 GRL_CORE_ERROR_LOAD_PLUGIN_FAILED,
-                 "Unknown module file for plugin with id '%s'", plugin_id);
-    return FALSE;
-  }
-
-  /* Check if module name is actually a full pathname */
-  if (g_path_is_absolute (module_name)) {
-    return grl_plugin_registry_load (registry, module_name, error);
-  }
-
-  if (g_str_has_suffix (module_name, "." G_MODULE_SUFFIX)) {
-    module_filename = g_strdup (module_name);
-  } else {
-    module_filename = g_strconcat (module_name, "." G_MODULE_SUFFIX, NULL);
-  }
-
-  /* Search the module in default directory paths */
-  for (plugin_dir = registry->priv->plugins_dir;
-       plugin_dir;
-       plugin_dir = g_slist_next (plugin_dir)) {
-    module_fullpathname = g_build_filename (plugin_dir->data,
-                                            module_filename,
-                                            NULL);
-    if (g_file_test (module_fullpathname, G_FILE_TEST_EXISTS)) {
-      result = grl_plugin_registry_load (registry, module_fullpathname, error);
-      g_free (module_fullpathname);
-      g_free (module_filename);
-      return result;
-    }
-    g_free (module_fullpathname);
-  }
-
-  g_free (module_filename);
-
-  GRL_WARNING ("Module file for plugin '%s' not found", plugin_id);
-  g_set_error (error,
-               GRL_CORE_ERROR,
-               GRL_CORE_ERROR_LOAD_PLUGIN_FAILED,
-               "Module file for plugin '%s' not found", plugin_id);
-
-  return FALSE;
+  return grl_plugin_registry_load (registry, plugin_info->filename, error);
 }
 
 /**
diff --git a/src/grl-plugin-registry.h b/src/grl-plugin-registry.h
index 56be7da..22ae36f 100644
--- a/src/grl-plugin-registry.h
+++ b/src/grl-plugin-registry.h
@@ -206,7 +206,7 @@ void grl_plugin_registry_add_directory (GrlPluginRegistry *registry,
                                         const gchar *path);
 
 gboolean grl_plugin_registry_load (GrlPluginRegistry *registry,
-                                   const gchar *path,
+                                   const gchar *library_filename,
                                    GError **error);
 
 gboolean grl_plugin_registry_load_directory (GrlPluginRegistry *registry,