operation-options: bypass checks for NULL GValues

grl_registry_metadata_key_clamp() is a helper to check if a given
@value is between @min and @max. Only @min and @max should be well
defined values to be compared with. We can simply bypass when @value
is NULL (which means, @value is not changed).

This comes with a unit test that shows issue #148 but also highlights
some other bugs around the code. Those will be discussed in its own
issue trackers.

Resolves: https://gitlab.gnome.org/GNOME/grilo/-/issues/148

2 files changed, 64 insertions, 0 deletions

diff --git a/src/grl-registry.c b/src/grl-registry.c
index 9caf9e3..c994ed2 100644
--- a/src/grl-registry.c
+++ b/src/grl-registry.c
@@ -2289,6 +2289,9 @@ grl_registry_metadata_key_get_limits(GrlRegistry *registry,
   return FALSE;
 }
 
+/* @max and @min are expected to be initialized with G_VALUE_INIT (non null)
+ * Returns TRUE if @value has changed
+ */
 G_GNUC_INTERNAL gboolean
 grl_registry_metadata_key_clamp(GrlRegistry *registry,
                                 GrlKeyID key,
@@ -2298,6 +2301,13 @@ grl_registry_metadata_key_clamp(GrlRegistry *registry,
 {
   const gchar *key_name;
 
+  g_return_val_if_fail (min != NULL, FALSE);
+  g_return_val_if_fail (max != NULL, FALSE);
+
+  if (value == NULL) {
+    return FALSE;
+  }
+
   key_name = key_id_handler_get_name (&registry->priv->key_id_handler, key);
   if (key_name) {
     GParamSpec *key_pspec;
diff --git a/tests/operations.c b/tests/operations.c
index 48f5793..87cc50c 100644
--- a/tests/operations.c
+++ b/tests/operations.c
@@ -72,6 +72,59 @@ range_filters_max_min_int(void)
   }
 }
 
+static void
+range_filters_max_min_null(void)
+{
+  GrlOperationOptions *options = grl_operation_options_new (NULL);
+  gboolean ret;
+  GValue value = G_VALUE_INIT;
+  GValue *max, *min;
+
+  g_test_bug ("148");
+
+  /* Before */
+  grl_operation_options_get_key_range_filter (options,
+                                              GRL_METADATA_KEY_ORIENTATION,
+                                              &min,
+                                              &max);
+  /* TODO: this is actually a bug. Should get default min/max for this metadata-key 0/359.
+   * Seems that grilo stores the range in GParamSpec but does not set it in the HashTable
+   * GrlOperationsOptions look at. */
+  g_assert_null(min);
+  g_assert_null(max);
+
+  /* Test MIN */
+  g_value_init (&value, G_TYPE_INT);
+  g_value_set_int (&value, 90);
+  ret = grl_operation_options_set_key_range_filter_value (options,
+                                                          GRL_METADATA_KEY_ORIENTATION,
+                                                          &value,
+                                                          NULL);
+  g_assert_true(ret);
+
+  grl_operation_options_get_key_range_filter (options, GRL_METADATA_KEY_ORIENTATION, &min, &max);
+  g_assert_cmpint(g_value_get_int (min), ==, 90);
+  // TODO: Same bug, as max was not set we receive NULL instead
+  g_assert_null(max);
+  g_value_unset(min);
+
+  /* Test MAX */
+  g_value_set_int (&value, 180);
+  ret = grl_operation_options_set_key_range_filter_value (options,
+                                                          GRL_METADATA_KEY_ORIENTATION,
+                                                          NULL,
+                                                          &value);
+  g_assert_true(ret);
+
+  grl_operation_options_get_key_range_filter (options, GRL_METADATA_KEY_ORIENTATION, &min, &max);
+  /* TODO: This is another bug. When we set max above, the min should not be changed.
+   * g_assert_cmpint(g_value_get_int (min), ==, 90);
+   */
+  g_assert_null(min);
+  g_assert_cmpint(g_value_get_int (max), ==, 180);
+  g_value_unset(max);
+}
+
 int
 main (int argc, char **argv)
 {
@@ -85,6 +138,7 @@ main (int argc, char **argv)
 
   /* registry tests */
   g_test_add_func ("/operation/range-filters/max-min/int", range_filters_max_min_int);
+  g_test_add_func ("/operation/range-filters/max-min/null", range_filters_max_min_null);
 
   return g_test_run ();
 }