[grilo] operation-options: Fix storing numeric limits

[Victor Toso <victortoso src gnome org>]

commit b50dc15f76e89838abde8403fe5f72b55d66496d
Author: Victor Toso <victortoso gnome org>
Date:   Fri Mar 19 23:39:49 2021 +0100

    operation-options: Fix storing numeric limits
    
    GrlOperationOptions needs to validate user input against the maximum
    and minimum values of a metadata-key of numeric type. For numeric
    types, the follow should always be true:
    
      @min_value_registered <= @min_value_user
      @min_value_user <= @max_value_user
      @max_value_user <= @max_value_registered.
    
    So, we do implement some sort generic CLAMP for numeric types.  Sadly,
    glib does not provide as of yet a g_value_cmp() [0] so we need to
    write a bit more code to make this checks in Grilo today.
    
    The followup commit adds a unit test.
    
    Related: https://gitlab.gnome.org/GNOME/grilo/-/issues/140
    
    [0] https://gitlab.gnome.org/GNOME/glib/-/issues/129

 src/grl-metadata-key.c      |  1 +
 src/grl-operation-options.c | 48 ++++++++++++++++++++++-------
 src/grl-registry-priv.h     | 11 +++++++
 src/grl-registry.c          | 75 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 124 insertions(+), 11 deletions(-)
---
diff --git a/src/grl-metadata-key.c b/src/grl-metadata-key.c
index ce601051..342b4f47 100644
--- a/src/grl-metadata-key.c
+++ b/src/grl-metadata-key.c
@@ -785,3 +785,4 @@ grl_metadata_key_list_new(GrlKeyID first_key, ...)
 
   return g_list_reverse (key_list);
 }
+
diff --git a/src/grl-operation-options.c b/src/grl-operation-options.c
index ec50d654..7296f477 100644
--- a/src/grl-operation-options.c
+++ b/src/grl-operation-options.c
@@ -32,7 +32,7 @@
 #include <grl-value-helper.h>
 #include <grl-range-value.h>
 #include <grl-log.h>
-#include <grl-registry.h>
+#include <grl-registry-priv.h>
 
 #include "grl-operation-options-priv.h"
 #include "grl-type-builtins.h"
@@ -705,6 +705,10 @@ grl_operation_options_get_key_filter_list (GrlOperationOptions *options)
  *
  * If @max_value is %NULL, then filter is "@key >= @min_value".
  *
+ * Note that @key will always respect the limits defined at creation time.
+ * e.g: Core's GRL_METADATA_KEY_ORIENTATION has "max = 359" and "min = 0",
+ * user can set "@max_value = 180" but "@max_value = 720" would be ignored.
+ *
  * Returns: %TRUE on success
  *
  * Since: 0.2.0
@@ -716,22 +720,44 @@ grl_operation_options_set_key_range_filter_value (GrlOperationOptions *options,
                                                   GValue *max_value)
 {
   gboolean ret;
+  GrlRegistry *registry;
+  GValue min_registered = G_VALUE_INIT;
+  GValue max_registered = G_VALUE_INIT;                                                \
+  gboolean min_changed = FALSE;
+  gboolean max_changed = FALSE;
 
   ret = (options->priv->caps == NULL) ||
     grl_caps_is_key_range_filter (options->priv->caps, key);
 
-  if (ret) {
-    if (min_value || max_value) {
-      grl_range_value_hashtable_insert (options->priv->key_range_filter,
-                                        GRLKEYID_TO_POINTER (key),
-                                        min_value, max_value);
-    } else {
-      g_hash_table_remove (options->priv->key_range_filter,
-                           GRLKEYID_TO_POINTER (key));
-    }
+  if (!ret) {
+      return FALSE;
   }
 
-  return ret;
+  if (!min_value && !max_value) {
+    g_hash_table_remove (options->priv->key_range_filter,
+                         GRLKEYID_TO_POINTER (key));
+    return TRUE;
+  }
+
+  /* Does a CLAMP for GValue of numeric types so new min and max values are
+   * within min-max values as registered per metadata-key */
+  registry = grl_registry_get_default ();
+  if (grl_registry_metadata_key_get_limits(registry, key, &min_registered, &max_registered)) {
+    max_changed = grl_registry_metadata_key_clamp(registry, key, &min_registered, max_value, 
&max_registered);
+    min_changed = grl_registry_metadata_key_clamp(registry, key, &min_registered, min_value, 
&max_registered);
+  } else {
+    GRL_DEBUG("Can't get limits of this key");
+  }
+
+  if (min_changed || max_changed) {
+    GRL_DEBUG("@min_value=%c @max_value=%c changes due metadata-key limits",
+              min_changed ? 'y':'n', max_changed ? 'y':'n');
+  }
+
+  grl_range_value_hashtable_insert (options->priv->key_range_filter,
+                                    GRLKEYID_TO_POINTER (key),
+                                    min_value, max_value);
+  return TRUE;
 }
 
 /**
diff --git a/src/grl-registry-priv.h b/src/grl-registry-priv.h
index a692f51c..2d6c2778 100644
--- a/src/grl-registry-priv.h
+++ b/src/grl-registry-priv.h
@@ -47,4 +47,15 @@ GrlKeyID grl_registry_register_or_lookup_metadata_key (GrlRegistry *registry,
                                                        const GValue *value,
                                                        GrlKeyID bind_key);
 
+gboolean grl_registry_metadata_key_get_limits(GrlRegistry *registry,
+                                              GrlKeyID key,
+                                              GValue *min,
+                                              GValue *max);
+
+gboolean grl_registry_metadata_key_clamp(GrlRegistry *registry,
+                                         GrlKeyID key,
+                                         GValue *min,
+                                         GValue *value,
+                                         GValue *max);
+
 #endif /* _GRL_REGISTRY_PRIV_H_ */
diff --git a/src/grl-registry.c b/src/grl-registry.c
index 12e0031d..51223e9c 100644
--- a/src/grl-registry.c
+++ b/src/grl-registry.c
@@ -2238,3 +2238,78 @@ bail:
 
   return ret;
 }
+
+
+G_GNUC_INTERNAL gboolean
+grl_registry_metadata_key_get_limits(GrlRegistry *registry,
+                                     GrlKeyID key,
+                                     GValue *min,
+                                     GValue *max)
+{
+  GParamSpec *key_pspec;
+  const gchar *key_name;
+  GType key_type;
+
+  key_name = key_id_handler_get_name (&registry->priv->key_id_handler, key);
+  if (!key_name) {
+    return FALSE;
+  }
+
+  key_pspec = g_hash_table_lookup (registry->priv->system_keys, key_name);
+  if (!key_pspec) {
+    return FALSE;
+  }
+
+  key_type = G_PARAM_SPEC_VALUE_TYPE (key_pspec);
+
+#define CHECK_NUMERIC_AND_SET_VALUE_LIMITS(value_type, numeric_type, getter, cast_type) { \
+  if (value_type == numeric_type) {                                                       \
+    g_value_init (min, numeric_type);                                                     \
+    g_value_init (max, numeric_type);                                                     \
+    g_value_set_##getter (min, (cast_type (key_pspec))->minimum);                         \
+    g_value_set_##getter (max, (cast_type (key_pspec))->maximum);                         \
+    return TRUE;                                                                          \
+  }                                                                                       \
+}
+
+  CHECK_NUMERIC_AND_SET_VALUE_LIMITS (key_type, G_TYPE_INT, int, G_PARAM_SPEC_INT);
+  CHECK_NUMERIC_AND_SET_VALUE_LIMITS (key_type, G_TYPE_LONG, long, G_PARAM_SPEC_LONG);
+  CHECK_NUMERIC_AND_SET_VALUE_LIMITS (key_type, G_TYPE_INT64, int64, G_PARAM_SPEC_INT64);
+  CHECK_NUMERIC_AND_SET_VALUE_LIMITS (key_type, G_TYPE_CHAR, schar, G_PARAM_SPEC_CHAR);
+  CHECK_NUMERIC_AND_SET_VALUE_LIMITS (key_type, G_TYPE_UINT, uint, G_PARAM_SPEC_UINT);
+  CHECK_NUMERIC_AND_SET_VALUE_LIMITS (key_type, G_TYPE_ULONG, ulong, G_PARAM_SPEC_ULONG);
+  CHECK_NUMERIC_AND_SET_VALUE_LIMITS (key_type, G_TYPE_UINT64, uint64, G_PARAM_SPEC_UINT64);
+  CHECK_NUMERIC_AND_SET_VALUE_LIMITS (key_type, G_TYPE_UCHAR, uchar, G_PARAM_SPEC_UCHAR);
+  CHECK_NUMERIC_AND_SET_VALUE_LIMITS (key_type, G_TYPE_FLOAT, float, G_PARAM_SPEC_FLOAT);
+  CHECK_NUMERIC_AND_SET_VALUE_LIMITS (key_type, G_TYPE_DOUBLE, double, G_PARAM_SPEC_DOUBLE);
+  return FALSE;
+}
+
+G_GNUC_INTERNAL gboolean
+grl_registry_metadata_key_clamp(GrlRegistry *registry,
+                                GrlKeyID key,
+                                GValue *min,
+                                GValue *value,
+                                GValue *max)
+{
+  const gchar *key_name;
+
+  key_name = key_id_handler_get_name (&registry->priv->key_id_handler, key);
+  if (key_name) {
+    GParamSpec *key_pspec;
+
+    key_pspec = g_hash_table_lookup (registry->priv->system_keys, key_name);
+    if (key_pspec) {
+      if (g_param_values_cmp(key_pspec, value, min) < 0) {
+        GRL_DEBUG("reset value to min");
+        g_value_transform(min, value);
+        return TRUE;
+      } else if (g_param_values_cmp(key_pspec, value, max) > 0) {
+        GRL_DEBUG("reset value to max");
+        g_value_transform(max, value);
+        return TRUE;
+      }
+    }
+  }
+  return FALSE;
+}