[grilo] core: remove optional info in plugins

[Juan A. Suárez Romero <jasuarez src gnome org>]

commit e0d59b36c7df7439b091510e7b52eac616d0b3c3
Author: Juan A. Suarez Romero <jasuarez igalia com>
Date:   Thu Dec 10 11:33:17 2015 +0000

    core: remove optional info in plugins
    
    Set only a restricted set of information in plugins.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=759295

 bindings/vala/grilo-0.3.vapi      |    7 +-
 src/grl-plugin-priv.h             |    9 +-
 src/grl-plugin.c                  |  147 +++++++++++++++----------------------
 src/grl-plugin.h                  |    7 +-
 src/grl-registry.c                |   66 ++++++++---------
 tools/grilo-inspect/grl-inspect.c |   23 ++----
 6 files changed, 105 insertions(+), 154 deletions(-)
---
diff --git a/bindings/vala/grilo-0.3.vapi b/bindings/vala/grilo-0.3.vapi
index 8c132df..81f7b0a 100644
--- a/bindings/vala/grilo-0.3.vapi
+++ b/bindings/vala/grilo-0.3.vapi
@@ -317,20 +317,17 @@ namespace Grl {
                public unowned string get_description ();
                public unowned string get_filename ();
                public unowned string get_id ();
-               public unowned string get_info (string key);
-               public GLib.List<weak string> get_info_keys ();
                public unowned string get_license ();
+               public unowned string get_module_name ();
                public unowned string get_name ();
                public unowned string get_site ();
                public GLib.List<weak Grl.Source> get_sources ();
                public unowned string get_version ();
-               public bool load (GLib.List<Grl.Config> configurations);
                public void register_keys ();
                public void set_filename (string filename);
                public void set_id (string id);
-               public void set_info (string key, string value);
                public void set_module (GLib.Module module);
-               public void set_optional_info (GLib.HashTable<void*,void*> info);
+               public void set_module_name (string module_name);
                public void unload ();
                [NoAccessorMethod]
                public bool loaded { get; }
diff --git a/src/grl-plugin-priv.h b/src/grl-plugin-priv.h
index 5da8dd3..0aa796b 100644
--- a/src/grl-plugin-priv.h
+++ b/src/grl-plugin-priv.h
@@ -39,8 +39,8 @@
 
 G_BEGIN_DECLS
 
-void grl_plugin_set_optional_info (GrlPlugin *plugin,
-                                   GHashTable *info);
+void grl_plugin_set_desc (GrlPlugin *plugin,
+                          GrlPluginDescriptor *desc);
 
 void grl_plugin_set_load_func (GrlPlugin *plugin,
                                GrlPluginInitFunc load_function);
@@ -66,9 +66,8 @@ void grl_plugin_set_filename (GrlPlugin *plugin,
 void grl_plugin_set_module (GrlPlugin *plugin,
                             GModule *module);
 
-void grl_plugin_set_info (GrlPlugin *plugin,
-                          const gchar *key,
-                          const gchar *value);
+void grl_plugin_set_module_name (GrlPlugin *plugin,
+                                 const gchar *module_name);
 
 G_END_DECLS
 
diff --git a/src/grl-plugin.c b/src/grl-plugin.c
index eaaa541..739a522 100644
--- a/src/grl-plugin.c
+++ b/src/grl-plugin.c
@@ -58,8 +58,8 @@ static GParamSpec *properties[PROP_LAST] = { 0 };
 struct _GrlPluginPrivate {
   GrlPluginDescriptor desc;
   gchar *filename;
+  gchar *module_name;
   GModule *module;
-  GHashTable *optional_info;
   gboolean loaded;
 };
 
@@ -109,10 +109,6 @@ static void
 grl_plugin_init (GrlPlugin *plugin)
 {
   plugin->priv = GRL_PLUGIN_GET_PRIVATE (plugin);
-  plugin->priv->optional_info = g_hash_table_new_full (g_str_hash,
-                                                       g_str_equal,
-                                                       g_free,
-                                                       g_free);
 }
 
 static void
@@ -120,9 +116,8 @@ grl_plugin_finalize (GObject *object)
 {
   GrlPlugin *plugin = GRL_PLUGIN (object);
 
-  g_free (plugin->priv->desc.id);
   g_free (plugin->priv->filename);
-  g_hash_table_unref (plugin->priv->optional_info);
+  g_free (plugin->priv->module_name);
 
   G_OBJECT_CLASS (grl_plugin_parent_class)->finalize (object);
 }
@@ -147,25 +142,34 @@ grl_plugin_get_property (GObject *object,
 
 /* ============ PRIVATE API ============ */
 
-/*
- * grl_plugin_set_optional_info:
+/**
+ * grl_plugin_set_desc: (skip)
  * @plugin: a plugin
- * @info: a hashtable containing optional information
+ * @desc: description
  *
- * Sets the optional information. Takes ownership of @info table.
- */
+ * Sets the plugin description.
+ **/
 void
-grl_plugin_set_optional_info (GrlPlugin *plugin,
-                              GHashTable *info)
+grl_plugin_set_desc (GrlPlugin *plugin,
+                     GrlPluginDescriptor *desc)
 {
-  g_return_if_fail (GRL_IS_PLUGIN (plugin));
-
-  g_hash_table_unref (plugin->priv->optional_info);
-  plugin->priv->optional_info = info;
+  plugin->priv->desc.major_version = desc->major_version;
+  plugin->priv->desc.minor_version = desc->minor_version;
+  plugin->priv->desc.id = desc->id;
+  plugin->priv->desc.name = desc->name;
+  plugin->priv->desc.description = desc->description;
+  plugin->priv->desc.author = desc->author;
+  plugin->priv->desc.version = desc->version;
+  plugin->priv->desc.license = desc->license;
+  plugin->priv->desc.site = desc->site;
+
+  plugin->priv->desc.init = desc->init;
+  plugin->priv->desc.deinit = desc->deinit;
+  plugin->priv->desc.register_keys = desc->register_keys;
 }
 
 /*
- * grl_plugin_set_load_func:
+ * grl_plugin_set_load_func: (skip)
  * @plugin: a plugin
  * @load_function: a function
  *
@@ -181,7 +185,7 @@ grl_plugin_set_load_func (GrlPlugin *plugin,
 }
 
 /*
- * grl_plugin_set_unload_func:
+ * grl_plugin_set_unload_func: (skip)
  * @plugin: a plugin
  * @unload_function: a function
  *
@@ -197,7 +201,7 @@ grl_plugin_set_unload_func (GrlPlugin *plugin,
 }
 
 /*
- * grl_plugin_set_register_keys_func:
+ * grl_plugin_set_register_keys_func: (skip)
  * @plugin: a plugin
  * @register_keys_function: a function
  *
@@ -214,7 +218,7 @@ grl_plugin_set_register_keys_func (GrlPlugin *plugin,
 }
 
 /**
- * grl_plugin_load:
+ * grl_plugin_load: (skip)
  * @plugin: a plugin
  * @configurations: (element-type GrlConfig): a list of configurations
  *
@@ -247,7 +251,7 @@ grl_plugin_load (GrlPlugin *plugin,
 }
 
 /*
- * grl_plugin_unload:
+ * grl_plugin_unload: (skip)
  * @plugin: a plugin
  *
  * Unloads the plugin
@@ -266,7 +270,7 @@ grl_plugin_unload (GrlPlugin *plugin)
 }
 
 /*
- * grl_plugin_register_keys:
+ * grl_plugin_register_keys: (skip)
  * @plugin: a plugin
  *
  * Register custom metadata keys for the plugin
@@ -286,7 +290,7 @@ grl_plugin_register_keys (GrlPlugin *plugin)
 }
 
 /*
- * grl_plugin_set_id:
+ * grl_plugin_set_id: (skip)
  * @plugin: a plugin
  * @id: plugin identifier
  *
@@ -304,7 +308,7 @@ grl_plugin_set_id (GrlPlugin *plugin,
 }
 
 /*
- * grl_plugin_set_filename:
+ * grl_plugin_set_filename: (skip)
  * @plugin: a plugin
  * @filename: a filename
  *
@@ -322,38 +326,36 @@ grl_plugin_set_filename (GrlPlugin *plugin,
 }
 
 /*
- * grl_plugin_set_module:
+ * grl_plugin_set_module_name: (skip)
  * @plugin: a plugin
- * @module: a module
+ * @module_name: a module name
  *
- * Sets the module of the plugin
+ * Sets the module name for the the plugin
  */
 void
-grl_plugin_set_module (GrlPlugin *plugin,
-                       GModule *module)
+grl_plugin_set_module_name (GrlPlugin *plugin,
+                            const gchar *module_name)
 {
   g_return_if_fail (GRL_IS_PLUGIN (plugin));
-  plugin->priv->module = module;
+
+  g_clear_pointer (&plugin->priv->module_name, g_free);
+
+  plugin->priv->module_name = g_strdup (module_name);
 }
 
 /*
- * grl_plugin_set_info:
+ * grl_plugin_set_module: (skip)
  * @plugin: a plugin
- * @key: key representing information about this plugin
- * @value: the information itself
+ * @module: a module
  *
- * Sets the information of the @plugin that is associaed with the given @key.
+ * Sets the module of the plugin
  */
 void
-grl_plugin_set_info (GrlPlugin *plugin,
-                     const gchar *key,
-                     const gchar *value)
+grl_plugin_set_module (GrlPlugin *plugin,
+                       GModule *module)
 {
   g_return_if_fail (GRL_IS_PLUGIN (plugin));
-
-  g_hash_table_insert (plugin->priv->optional_info,
-                       g_strdup (key),
-                       g_strdup (value));
+  plugin->priv->module = module;
 }
 
 /* ============ PUBLIC API ============= */
@@ -503,70 +505,37 @@ grl_plugin_get_filename (GrlPlugin *plugin)
 }
 
 /**
- * grl_plugin_get_module: (skip)
+ * grl_plugin_get_module_name:
  * @plugin: a plugin
  *
- * Gets the #GModule containing the plugin
+ * Get the plugin module name
  *
- * Returns: a #GModule
- *
- * Since: 0.2.0
- **/
-GModule *
-grl_plugin_get_module (GrlPlugin *plugin)
+ * Returns: the module name containing @plugin
+ */
+const gchar *
+grl_plugin_get_module_name (GrlPlugin *plugin)
 {
   g_return_val_if_fail (GRL_IS_PLUGIN (plugin), NULL);
 
-  return plugin->priv->module;
+  return plugin->priv->module_name;
 }
 
 /**
- * grl_plugin_get_info_keys:
+ * grl_plugin_get_module: (skip)
  * @plugin: a plugin
  *
- * Returns a list of keys that can be queried to retrieve information about the
- * plugin.
+ * Gets the #GModule containing the plugin
  *
- * Returns: (transfer container) (element-type utf8):
- * a #GList of strings containing the keys. The content of the list is
- * owned by the plugin and should not be modified or freed. Use g_list_free()
- * when done using the list.
+ * Returns: a #GModule
  *
  * Since: 0.2.0
  **/
-GList *
-grl_plugin_get_info_keys (GrlPlugin *plugin)
-{
-  g_return_val_if_fail (GRL_IS_PLUGIN (plugin), NULL);
-
-  if (plugin->priv->optional_info) {
-    return g_hash_table_get_keys (plugin->priv->optional_info);
-  } else {
-    return NULL;
-  }
-}
-
-/**
- * grl_plugin_get_info:
- * @plugin: a plugin
- * @key: a key representing information about this plugin
- *
- * Get the information of the @plugin that is associated with the given key
- *
- * Returns: the information assigned to the given @key or NULL if there is no such information
- *
- * Since: 0.2.0
- */
-const gchar *
-grl_plugin_get_info (GrlPlugin *plugin, const gchar *key)
+GModule *
+grl_plugin_get_module (GrlPlugin *plugin)
 {
   g_return_val_if_fail (GRL_IS_PLUGIN (plugin), NULL);
 
-  if (!plugin->priv->optional_info) {
-    return NULL;
-  }
-
-  return g_hash_table_lookup (plugin->priv->optional_info, key);
+  return plugin->priv->module;
 }
 
 /**
diff --git a/src/grl-plugin.h b/src/grl-plugin.h
index 2b135b8..5273d6e 100644
--- a/src/grl-plugin.h
+++ b/src/grl-plugin.h
@@ -123,12 +123,9 @@ const gchar *grl_plugin_get_id (GrlPlugin *plugin);
 
 const gchar *grl_plugin_get_filename (GrlPlugin *plugin);
 
-GModule *grl_plugin_get_module (GrlPlugin *plugin);
-
-GList *grl_plugin_get_info_keys (GrlPlugin *plugin);
+const gchar *grl_plugin_get_module_name (GrlPlugin *plugin);
 
-const gchar *grl_plugin_get_info (GrlPlugin *plugin,
-                                  const gchar *key);
+GModule *grl_plugin_get_module (GrlPlugin *plugin);
 
 GList *grl_plugin_get_sources (GrlPlugin *plugin);
 
diff --git a/src/grl-registry.c b/src/grl-registry.c
index 56c8600..6a7d001 100644
--- a/src/grl-registry.c
+++ b/src/grl-registry.c
@@ -564,7 +564,7 @@ grl_registry_preload_plugin (GrlRegistry *registry,
   gchar *module_filename;
   gchar *module_fullpathname;
 
-  if ((suffix = g_strrstr (plugin_info_filename, "." GRL_PLUGIN_INFO_SUFFIX)) == NULL) {
+  if ((suffix = g_strrstr (plugin_info_filename, "." G_MODULE_SUFFIX)) == NULL) {
     return NULL;
   }
 
@@ -593,8 +593,7 @@ grl_registry_preload_plugin (GrlRegistry *registry,
   if (info) {
     plugin = g_object_new (GRL_TYPE_PLUGIN, NULL);
     grl_plugin_set_id (plugin, id);
-    grl_plugin_set_optional_info (plugin, info);
-    module_name = grl_plugin_get_info (plugin, GRL_PLUGIN_INFO_MODULE);
+    module_name = g_hash_table_lookup (info, GRL_PLUGIN_INFO_MODULE);
     if (!module_name) {
       GRL_WARNING ("Unknown module file for plugin with id '%s'", id);
       g_object_unref (plugin);
@@ -632,6 +631,7 @@ grl_registry_preload_plugins_directory (GrlRegistry *registry,
   GError *error = NULL;
   const gchar *entry;
   GrlPlugin *plugin;
+  gchar *filename;
 
   dir = g_dir_open (directory, 0, &error);
   if (!dir) {
@@ -643,7 +643,9 @@ grl_registry_preload_plugins_directory (GrlRegistry *registry,
   }
 
   while ((entry = g_dir_read_name (dir)) != NULL) {
-    plugin = grl_registry_preload_plugin (registry, directory, entry);
+    filename = g_build_filename (directory, entry, NULL);
+    plugin = grl_registry_prepare_plugin (registry, filename, NULL);
+    g_free (filename);
     if (plugins_loaded && plugin) {
       *plugins_loaded = g_list_prepend (*plugins_loaded, plugin);
     }
@@ -1222,7 +1224,7 @@ grl_registry_prepare_plugin_from_desc (GrlRegistry *registry,
   grl_plugin_set_register_keys_func (plugin, plugin_desc->register_keys);
 
   /* Insert plugin ID as part of plugin information */
-  grl_plugin_set_info (plugin, GRL_PLUGIN_INFO_MODULE, plugin_desc->id);
+  grl_plugin_set_module_name (plugin, plugin_desc->id);
 
   return plugin;
 }
@@ -1276,23 +1278,12 @@ grl_registry_prepare_plugin (GrlRegistry *registry,
   plugin = g_hash_table_lookup (registry->priv->plugins,
                                 plugin_desc->id);
 
-  if (!plugin) {
-    info_dirname = g_path_get_dirname (library_filename);
-    info_filename = g_strconcat (plugin_desc->id, "." GRL_PLUGIN_INFO_SUFFIX, NULL);
-    plugin = grl_registry_preload_plugin (registry, info_dirname, info_filename);
-    g_free (info_dirname);
-    g_free (info_filename);
-    if (!plugin) {
-      g_set_error (error,
-                   GRL_CORE_ERROR,
-                   GRL_CORE_ERROR_LOAD_PLUGIN_FAILED,
-                   _("Unable to load plugin '%s'"), plugin_desc->id);
-      g_module_close (module);
-      return NULL;
-    }
-  } else {
-    /* Check if the existent plugin is for a different module */
-    if (g_strcmp0 (grl_plugin_get_filename (plugin), library_filename) != 0) {
+  if (plugin) {
+    g_module_close (module);
+    /* Check if the existent plugin is precisely this same plugin */
+    if (g_strcmp0 (grl_plugin_get_filename (plugin), library_filename == 0)) {
+      return plugin;
+    } else {
       GRL_WARNING ("Plugin '%s' already exists", library_filename);
       g_set_error (error,
                    GRL_CORE_ERROR,
@@ -1302,21 +1293,28 @@ grl_registry_prepare_plugin (GrlRegistry *registry,
     }
   }
 
-  if (!grl_plugin_get_module (plugin)) {
-    grl_plugin_set_load_func (plugin, plugin_desc->init);
-    grl_plugin_set_unload_func (plugin, plugin_desc->deinit);
-    grl_plugin_set_register_keys_func (plugin, plugin_desc->register_keys);
+  /* Check if plugin is allowed */
+  if (registry->priv->allowed_plugins &&
+      !g_slist_find_custom (registry->priv->allowed_plugins,
+                            plugin_desc->id,
+                            (GCompareFunc) g_strcmp0)) {
+    GRL_DEBUG ("Plugin '%s' not allowed; skipping", plugin_desc->id);
+    g_module_close (module);
+    return NULL;
+  }
+
+  plugin = g_object_new (GRL_TYPE_PLUGIN, NULL);
+  grl_plugin_set_desc (plugin, plugin_desc);
+  grl_plugin_set_module (plugin, module);
+  grl_plugin_set_filename (plugin, library_filename);
 
-    /* Insert module name as part of plugin information */
-    module_name = g_path_get_basename (library_filename);
-    grl_plugin_set_info (plugin, GRL_PLUGIN_INFO_MODULE, module_name);
-    g_free (module_name);
+  /* Make plugin resident */
+  g_module_make_resident (module);
 
-    grl_plugin_set_module (plugin, module);
+  g_hash_table_insert (registry->priv->plugins, g_strdup (plugin_desc->id), plugin);
 
-    /* Make plugin resident */
-    g_module_make_resident (module);
-  }
+  /* Register custom keys */
+  grl_plugin_register_keys (plugin);
 
   return plugin;
 }
diff --git a/tools/grilo-inspect/grl-inspect.c b/tools/grilo-inspect/grl-inspect.c
index 13dfe62..47610b2 100644
--- a/tools/grilo-inspect/grl-inspect.c
+++ b/tools/grilo-inspect/grl-inspect.c
@@ -296,15 +296,11 @@ print_version (void)
 static void
 introspect_source (const gchar *source_id)
 {
-  GList *info_key;
-  GList *info_keys;
   GrlMediaType supported_media;
   GrlPlugin *plugin;
   GrlSource *source;
   GrlSupportedOps supported_ops;
   const gchar **tags;
-  const gchar *value;
-  gchar *key;
 
   source = grl_registry_lookup_source (registry, source_id);
 
@@ -314,18 +310,13 @@ introspect_source (const gchar *source_id)
     if (plugin) {
       g_print ("Plugin Details:\n");
       g_print ("  %-20s %s\n", "Identifier:", grl_plugin_get_id (plugin));
-      g_print ("  %-20s %s\n", "Filename:",
-               grl_plugin_get_filename (plugin));
-
-      info_keys = grl_plugin_get_info_keys (plugin);
-      for (info_key = info_keys; info_key; info_key = g_list_next (info_key)) {
-        key = g_strdup_printf ("%s:", (gchar *) info_key->data);
-        key[0] = g_ascii_toupper (key[0]);
-        value = grl_plugin_get_info (plugin, info_key->data);
-        g_print ("  %-20s %s\n", key, value);
-        g_free (key);
-      }
-      g_list_free (info_keys);
+      g_print ("  %-20s %s\n", "Name:", grl_plugin_get_name (plugin));
+      g_print ("  %-20s %s\n", "Description:", grl_plugin_get_description (plugin));
+      g_print ("  %-20s %s\n", "Filename:", grl_plugin_get_filename (plugin));
+      g_print ("  %-20s %s\n", "Author:", grl_plugin_get_author (plugin));
+      g_print ("  %-20s %s\n", "Version:", grl_plugin_get_version (plugin));
+      g_print ("  %-20s %s\n", "License:", grl_plugin_get_license (plugin));
+      g_print ("  %-20s %s\n", "Site:", grl_plugin_get_site (plugin));
       g_print ("\n");
     }