[core 1/7] core: Do not allow empty keys

["Juan A. Suarez Romero" <jasuarez igalia com>]

When adding a new key into GrlData or GrlRelatedKeys, the key must have
at least one value.

Signed-off-by: Juan A. Suarez Romero <jasuarez igalia com>
---
 src/data/grl-data.c         |   46 +++++++++++++-------------
 src/data/grl-data.h         |    2 +-
 src/data/grl-related-keys.c |   73 ++++++++++++++++++------------------------
 src/data/grl-related-keys.h |    7 ++--
 4 files changed, 58 insertions(+), 70 deletions(-)

diff --git a/src/data/grl-data.c b/src/data/grl-data.c
index 6e3461e..7a0a988 100644
--- a/src/data/grl-data.c
+++ b/src/data/grl-data.c
@@ -257,6 +257,10 @@ grl_data_set (GrlData *data, GrlKeyID key, const GValue *value)
   g_return_if_fail (GRL_IS_DATA (data));
   g_return_if_fail (key);
 
+  if (!value) {
+    return;
+  }
+
   /* Get the right set of related keys */
   if (grl_data_length (data, key) > 0) {
     relkeys = grl_data_get_related_keys (data, key, 0);
@@ -296,14 +300,13 @@ grl_data_set_string (GrlData *data,
                      GrlKeyID key,
                      const gchar *strvalue)
 {
+  GValue value = { 0 };
+
   if (strvalue) {
-    GValue value = { 0 };
     g_value_init (&value, G_TYPE_STRING);
     g_value_set_string (&value, strvalue);
     grl_data_set (data, key, &value);
     g_value_unset (&value);
-  } else {
-    grl_data_set (data, key, NULL);
   }
 }
 
@@ -440,6 +443,10 @@ grl_data_set_binary (GrlData *data, GrlKeyID key, const guint8 *buf, gsize size)
   GValue v = { 0 };
   GByteArray * array;
 
+  if (!buf || !size) {
+    return;
+  }
+
   array = g_byte_array_append(g_byte_array_sized_new(size),
 		              buf,
 		              size);
@@ -494,9 +501,8 @@ grl_data_get_binary(GrlData *data, GrlKeyID key, gsize *size)
 void
 grl_data_add (GrlData *data, GrlKeyID key)
 {
-  if (!grl_data_has_key (data, key)) {
-    grl_data_set (data, key, NULL);
-  }
+  GRL_WARNING ("grl_data_add() is deprecated. Added keys require a value. "
+               "Use instead grl_data_set()");
 }
 
 /**
@@ -637,7 +643,7 @@ grl_data_add_related_keys (GrlData *data,
   g_return_if_fail (GRL_IS_DATA (data));
   g_return_if_fail (GRL_IS_RELATED_KEYS (relkeys));
 
-  keys = grl_related_keys_get_keys (relkeys, TRUE);
+  keys = grl_related_keys_get_keys (relkeys);
   if (!keys) {
     /* Ignore empty set of related keys */
     GRL_WARNING ("Trying to add an empty GrlRelatedKeys to GrlData");
@@ -666,9 +672,6 @@ grl_data_add_related_keys (GrlData *data,
  *
  * Appends a new string value for @key in @data.
  *
- * If there are other keys that are related to @key, %NULL values will be
- * appended for each of them too.
- *
  * Since: 0.1.10
  **/
 void
@@ -678,9 +681,11 @@ grl_data_add_string (GrlData *data,
 {
   GrlRelatedKeys *relkeys;
 
-  relkeys = grl_related_keys_new ();
-  grl_related_keys_set_string (relkeys, key, strvalue);
-  grl_data_add_related_keys (data, relkeys);
+  if (strvalue) {
+    relkeys = grl_related_keys_new ();
+    grl_related_keys_set_string (relkeys, key, strvalue);
+    grl_data_add_related_keys (data, relkeys);
+  }
 }
 
 /**
@@ -691,9 +696,6 @@ grl_data_add_string (GrlData *data,
  *
  * Appends a new int value for @key in @data.
  *
- * If there are other keys that are related to @key, %NULL values will be
- * appended for each of them too.
- *
  * Since: 0.1.10
  **/
 void
@@ -716,9 +718,6 @@ grl_data_add_int (GrlData *data,
  *
  * Appends a new float value for @key in @data.
  *
- * If there are other keys that are related to @key, %NULL values will be
- * appended for each of them too.
- *
  * Since: 0.1.10
  **/
 void
@@ -742,9 +741,6 @@ grl_data_add_float (GrlData *data,
  *
  * Appends a new binary value for @key in @data.
  *
- * If there are other keys that are related to @key, %NULL values will be
- * appended for each of them too.
- *
  * Since: 0.1.10
  **/
 void
@@ -755,6 +751,10 @@ grl_data_add_binary (GrlData *data,
 {
   GrlRelatedKeys *relkeys;
 
+  if (!buf || !size) {
+    return;
+  }
+
   relkeys = grl_related_keys_new ();
   grl_related_keys_set_binary (relkeys, key, buf, size);
   grl_data_add_related_keys (data, relkeys);
@@ -972,7 +972,7 @@ grl_data_set_related_keys (GrlData *data,
   g_return_if_fail (GRL_IS_DATA (data));
   g_return_if_fail (GRL_IS_RELATED_KEYS (relkeys));
 
-  keys = grl_related_keys_get_keys (relkeys, TRUE);
+  keys = grl_related_keys_get_keys (relkeys);
   if (!keys) {
     GRL_WARNING ("Trying to set an empty GrlRelatedKeys into GrlData");
     g_object_unref (relkeys);
diff --git a/src/data/grl-data.h b/src/data/grl-data.h
index 2854162..9c646ff 100644
--- a/src/data/grl-data.h
+++ b/src/data/grl-data.h
@@ -118,7 +118,7 @@ gfloat grl_data_get_float (GrlData *data, GrlKeyID key);
 
 const guint8 *grl_data_get_binary(GrlData *data, GrlKeyID key, gsize *size);
 
-void grl_data_add (GrlData *data, GrlKeyID key);
+G_GNUC_DEPRECATED void grl_data_add (GrlData *data, GrlKeyID key);
 
 void grl_data_remove (GrlData *data, GrlKeyID key);
 
diff --git a/src/data/grl-related-keys.c b/src/data/grl-related-keys.c
index 5baaa97..14a9681 100644
--- a/src/data/grl-related-keys.c
+++ b/src/data/grl-related-keys.c
@@ -240,20 +240,23 @@ grl_related_keys_set (GrlRelatedKeys *relkeys,
   g_return_if_fail (GRL_IS_RELATED_KEYS (relkeys));
   g_return_if_fail (key);
 
+  if (!value) {
+    return;
+  }
+
   /* Dup value */
-  if (value) {
-    if (G_VALUE_TYPE (value) == GRL_METADATA_KEY_GET_TYPE (key)) {
-      copy = g_new0 (GValue, 1);
-      g_value_init (copy, G_VALUE_TYPE (value));
-      g_value_copy (value, copy);
-    } else {
-      GRL_WARNING ("value has type %s, but expected %s",
-                   g_type_name (G_VALUE_TYPE (value)),
-                   g_type_name (GRL_METADATA_KEY_GET_TYPE (key)));
-    }
+  if (G_VALUE_TYPE (value) != GRL_METADATA_KEY_GET_TYPE (key)) {
+    GRL_WARNING ("value has type %s, but expected %s",
+                 g_type_name (G_VALUE_TYPE (value)),
+                 g_type_name (GRL_METADATA_KEY_GET_TYPE (key)));
+    return;
   }
 
-  if (copy && g_param_value_validate (key, copy)) {
+  copy = g_new0 (GValue, 1);
+  g_value_init (copy, G_VALUE_TYPE (value));
+  g_value_copy (value, copy);
+
+  if (g_param_value_validate (key, copy)) {
     GRL_WARNING ("'%s' value invalid, adjusting",
                  GRL_METADATA_KEY_GET_NAME (key));
   }
@@ -276,14 +279,13 @@ grl_related_keys_set_string (GrlRelatedKeys *relkeys,
                              GrlKeyID key,
                              const gchar *strvalue)
 {
+  GValue value = { 0 };
+
   if (strvalue) {
-    GValue value = { 0 };
     g_value_init (&value, G_TYPE_STRING);
     g_value_set_string (&value, strvalue);
     grl_related_keys_set (relkeys, key, &value);
     g_value_unset (&value);
-  } else {
-    grl_related_keys_set (relkeys, key, NULL);
   }
 }
 
@@ -428,6 +430,10 @@ grl_related_keys_set_binary (GrlRelatedKeys *relkeys,
   GValue v = { 0 };
   GByteArray *array;
 
+  if (!buf || !size) {
+    return;
+  }
+
   array = g_byte_array_append (g_byte_array_sized_new(size),
                                buf,
                                size);
@@ -486,9 +492,8 @@ void
 grl_related_keys_add (GrlRelatedKeys *relkeys,
                       GrlKeyID key)
 {
-  if (!grl_related_keys_has_key (relkeys, key)) {
-    grl_related_keys_set (relkeys, key, NULL);
-  }
+  GRL_WARNING ("grl_related_keys_add() is deprecated. Added key requires a value. "
+               "Use instead grl_related_keys_set()");
 }
 
 /**
@@ -514,33 +519,21 @@ grl_related_keys_has_key (GrlRelatedKeys *relkeys,
 /**
  * grl_related_keys_get_keys:
  * @relkeys: set of related keys to inspect
- * @include_unknown: %TRUE if keys with no value must be included
  *
- * Returns a list with keys contained in @relkeys. If @include_unknown is
- * %FALSE, only those keys in @relkeys that have actually a value will be
- * returned.
+ * Returns a list with keys contained in @relkeys.
  *
- * Returns: (transfer container) (element-type GObject.ParamSpec): an array
- * with the keys. The content of the list should not be modified or freed. Use
+ * Returns: (transfer container) (element-type GObject.ParamSpec): a list with
+ * the keys. The content of the list should not be modified or freed. Use
  * g_list_free() when done using the list.
  *
  * Since: 0.1.10
  **/
 GList *
-grl_related_keys_get_keys (GrlRelatedKeys *relkeys,
-                           gboolean include_unknown)
+grl_related_keys_get_keys (GrlRelatedKeys *relkeys)
 {
-  GList *keylist;
-
   g_return_val_if_fail (GRL_IS_RELATED_KEYS (relkeys), NULL);
 
-  keylist = g_hash_table_get_keys (relkeys->priv->data);
-
-  if (!include_unknown) {
-    keylist = g_list_remove_all (keylist, NULL);
-  }
-
-  return keylist;
+  return g_hash_table_get_keys (relkeys->priv->data);
 }
 
 /**
@@ -598,16 +591,12 @@ grl_related_keys_dup (GrlRelatedKeys *relkeys)
 
   dup_relkeys = grl_related_keys_new ();
 
-  keys = grl_related_keys_get_keys (relkeys, TRUE);
+  keys = grl_related_keys_get_keys (relkeys);
   for (key = keys; key; key = g_list_next (key)) {
     value = grl_related_keys_get (relkeys, key->data);
-    if (value) {
-      value_copy = g_new0 (GValue, 1);
-      g_value_init (value_copy, G_VALUE_TYPE (value));
-      g_value_copy (value, value_copy);
-    } else {
-      value_copy = NULL;
-    }
+    value_copy = g_new0 (GValue, 1);
+    g_value_init (value_copy, G_VALUE_TYPE (value));
+    g_value_copy (value, value_copy);
     g_hash_table_insert (dup_relkeys->priv->data, key->data, value_copy);
   }
 
diff --git a/src/data/grl-related-keys.h b/src/data/grl-related-keys.h
index 659ccfb..ef5eb17 100644
--- a/src/data/grl-related-keys.h
+++ b/src/data/grl-related-keys.h
@@ -134,14 +134,13 @@ const guint8 *grl_related_keys_get_binary(GrlRelatedKeys *relkeys,
                                           GrlKeyID key,
                                           gsize *size);
 
-void grl_related_keys_add (GrlRelatedKeys *relkeys,
-                           GrlKeyID key);
+G_GNUC_DEPRECATED void grl_related_keys_add (GrlRelatedKeys *relkeys,
+                                            GrlKeyID key);
 
 gboolean grl_related_keys_has_key (GrlRelatedKeys *relkeys,
                                    GrlKeyID key);
 
-GList *grl_related_keys_get_keys (GrlRelatedKeys *relkeys,
-                                  gboolean include_unknown);
+GList *grl_related_keys_get_keys (GrlRelatedKeys *relkeys);
 
 gboolean grl_related_keys_key_is_known (GrlRelatedKeys *relkeys,
                                         GrlKeyID key);
-- 
1.7.4