[librest/gwagner/deprecated: 10/10] params: reworked to boxed and list

[Günther Wagner <gwagner src gnome org>]

commit f633f6bd2795df3472fc2000853d9462ca680c2c
Author: Günther Wagner <info gunibert de>
Date:   Tue Dec 28 16:46:10 2021 +0100

    params: reworked to boxed and list
    
    RestParams was implemented as HashTable. Limitations are that it did
    not preserved the order of individual parameters aswell as duplicates
    aren't allowed. Reworked it to a GList and introduced reference counting
    and a boxed type.

 rest-extras/meson.build |   3 +
 rest/meson.build        |   6 +-
 rest/rest-params.c      | 196 ++++++++++++++++++++++++++++++++----------------
 rest/rest-params.h      |  52 ++++++++-----
 rest/rest-proxy-call.c  |   2 +-
 tests/meson.build       |   7 ++
 tests/params.c          | 141 ++++++++++++++++++++++++++++++++++
 tests/test-media.png    | Bin 0 -> 2509 bytes
 8 files changed, 323 insertions(+), 84 deletions(-)
---
diff --git a/rest-extras/meson.build b/rest-extras/meson.build
index 1ad9ee1..cf14ee8 100644
--- a/rest-extras/meson.build
+++ b/rest-extras/meson.build
@@ -49,9 +49,12 @@ if get_option('introspection')
   librest_extras_gir = gnome.generate_gir(librest_extras_lib,
     sources: librest_extras_sources + librest_extras_headers,
     namespace: 'RestExtras',
+    symbol_prefix: 'rest_extras',
+    identifier_prefix: 'RestExtras',
     nsversion: librest_api_version,
     includes: [ 'GObject-2.0', 'Gio-2.0', 'Soup-@0@'.format(libsoup_api_version), librest_gir[0] ],
     extra_args: [ '--accept-unprefixed' ],
+    dependencies: librest_extras_deps,
     install: true,
   )
 endif
diff --git a/rest/meson.build b/rest/meson.build
index 396ec83..127e578 100644
--- a/rest/meson.build
+++ b/rest/meson.build
@@ -77,9 +77,11 @@ if get_option('introspection')
   endforeach
 
   librest_gir = gnome.generate_gir(librest_lib,
-    sources: [ librest_headers, librest_enums[1] ],
-    namespace: 'Rest',
+    sources: librest_sources + librest_headers + librest_enums,
     nsversion: librest_api_version,
+    namespace: 'Rest',
+    symbol_prefix: 'rest',
+    identifier_prefix: 'Rest',
     includes: [ 'GObject-2.0', 'Gio-2.0', 'Soup-@0@'.format(libsoup_api_version) ],
     extra_args: librest_gir_extra_args,
     install: true,
diff --git a/rest/rest-params.c b/rest/rest-params.c
index f246cc2..f2ac286 100644
--- a/rest/rest-params.c
+++ b/rest/rest-params.c
@@ -30,18 +30,7 @@
  * @see_also: #RestParam, #RestProxyCall.
  */
 
-/*
- * RestParams is an alias for GHashTable achieved by opaque types in the public
- * headers and casting internally. This has several limitations, mainly
- * supporting multiple parameters with the same name and preserving the ordering
- * of parameters.
- *
- * These are not requirements for the bulk of the web services, but this
- * limitation does mean librest can't be used for a few web services.
- *
- * TODO: this should be a list to support multiple parameters with the same
- * name.
- */
+G_DEFINE_BOXED_TYPE (RestParams, rest_params, rest_params_ref, rest_params_unref)
 
 /**
  * rest_params_new:
@@ -53,11 +42,13 @@
 RestParams *
 rest_params_new (void)
 {
-  /* The key is a string that is owned by the RestParam, so we don't need to
-     explicitly free it on removal. */
-  return (RestParams *)
-    g_hash_table_new_full (g_str_hash, g_str_equal,
-                           NULL, (GDestroyNotify)rest_param_unref);
+  RestParams *self;
+
+  self = g_slice_new0 (RestParams);
+  self->ref_count = 1;
+  self->params = NULL;
+
+  return self;
 }
 
 /**
@@ -67,13 +58,73 @@ rest_params_new (void)
  * Destroy the #RestParams and the #RestParam objects that it contains.
  **/
 void
-rest_params_free (RestParams *params)
+rest_params_free (RestParams *self)
 {
-  GHashTable *hash = (GHashTable *)params;
+  g_assert (self);
+  g_assert_cmpint (self->ref_count, ==, 0);
 
-  g_return_if_fail (params);
+  g_list_free_full (g_steal_pointer (&self->params), (GDestroyNotify) rest_param_unref);
+
+  g_slice_free (RestParams, self);
+}
+
+/**
+ * rest_params_copy:
+ * @self: a #RestParams
+ *
+ * Makes a deep copy of a #RestParams.
+ *
+ * Returns: (transfer full): A newly created #RestParams with the same
+ *   contents as @self
+ */
+RestParams *
+rest_params_copy (RestParams *self)
+{
+  RestParams *copy;
+
+  g_return_val_if_fail (self, NULL);
+  g_return_val_if_fail (self->ref_count, NULL);
+
+  copy = rest_params_new ();
+  copy->params = g_list_copy_deep (self->params, (GCopyFunc) rest_param_ref, NULL);
+
+  return copy;
+}
+
+/**
+ * rest_params_ref:
+ * @self: A #RestParams
+ *
+ * Increments the reference count of @self by one.
+ *
+ * Returns: (transfer full): @self
+ */
+RestParams *
+rest_params_ref (RestParams *self)
+{
+  g_return_val_if_fail (self, NULL);
+  g_return_val_if_fail (self->ref_count, NULL);
+
+  g_atomic_int_inc (&self->ref_count);
 
-  g_hash_table_destroy (hash);
+  return self;
+}
+
+/**
+ * rest_params_unref:
+ * @self: A #RestParams
+ *
+ * Decrements the reference count of @self by one, freeing the structure when
+ * the reference count reaches zero.
+ */
+void
+rest_params_unref (RestParams *self)
+{
+  g_return_if_fail (self);
+  g_return_if_fail (self->ref_count);
+
+  if (g_atomic_int_dec_and_test (&self->ref_count))
+    rest_params_free (self);
 }
 
 /**
@@ -84,14 +135,25 @@ rest_params_free (RestParams *params)
  * Add @param to @params.
  **/
 void
-rest_params_add (RestParams *params, RestParam *param)
+rest_params_add (RestParams *self,
+                 RestParam  *param)
 {
-  GHashTable *hash = (GHashTable *)params;
-
-  g_return_if_fail (params);
+  g_return_if_fail (self);
   g_return_if_fail (param);
 
-  g_hash_table_replace (hash, (gpointer)rest_param_get_name (param), param);
+  self->params = g_list_append (self->params, param);
+}
+
+static gint
+rest_params_find (gconstpointer self,
+                  gconstpointer name)
+{
+  const RestParam *e = self;
+  const char *n = name;
+  const char *n2 = rest_param_get_name ((RestParam *)e);
+
+  if (g_strcmp0 (n2, n) == 0) return 0;
+  return -1;
 }
 
 /**
@@ -105,14 +167,13 @@ rest_params_add (RestParams *params, RestParam *param)
  * doesn't exist
  **/
 RestParam *
-rest_params_get (RestParams *params, const char *name)
+rest_params_get (RestParams *self,
+                 const char *name)
 {
-  GHashTable *hash = (GHashTable *)params;
-
-  g_return_val_if_fail (params, NULL);
+  g_return_val_if_fail (self, NULL);
   g_return_val_if_fail (name, NULL);
 
-  return g_hash_table_lookup (hash, name);
+  return g_list_find_custom (self->params, name, rest_params_find)->data;
 }
 
 /**
@@ -123,14 +184,16 @@ rest_params_get (RestParams *params, const char *name)
  * Remove the #RestParam called @name.
  **/
 void
-rest_params_remove (RestParams *params, const char *name)
+rest_params_remove (RestParams *self,
+                    const char *name)
 {
-  GHashTable *hash = (GHashTable *)params;
+  GList *elem = NULL;
 
-  g_return_if_fail (params);
+  g_return_if_fail (self);
   g_return_if_fail (name);
 
-  g_hash_table_remove (hash, name);
+  elem = g_list_find_custom (self->params, name, rest_params_find);
+  self->params = g_list_remove (self->params, elem->data);
 }
 
 /**
@@ -143,27 +206,22 @@ rest_params_remove (RestParams *params, const char *name)
  * Returns: %TRUE if all of the parameters are simple strings, %FALSE otherwise.
  **/
 gboolean
-rest_params_are_strings (RestParams *params)
+rest_params_are_strings (RestParams *self)
 {
-  GHashTable *hash = (GHashTable *)params;
-  GHashTableIter iter;
-  RestParam *param;
-
-  g_return_val_if_fail (params, FALSE);
+  g_return_val_if_fail (self, FALSE);
 
-  g_hash_table_iter_init (&iter, hash);
-  while (g_hash_table_iter_next (&iter, NULL, (gpointer)&param)) {
-    if (!rest_param_is_string (param))
-      return FALSE;
-  }
+  for (GList *cur = self->params; cur; cur = g_list_next (cur))
+    {
+      if (!rest_param_is_string (cur->data))
+        return FALSE;
+    }
 
   return TRUE;
-
 }
 
 /**
  * rest_params_as_string_hash_table:
- * @params: a valid #RestParams
+ * @self: a valid #RestParams
  *
  * Create a new #GHashTable which contains the name and value of all string
  * (content type of text/plain) parameters.
@@ -174,23 +232,19 @@ rest_params_are_strings (RestParams *params)
  * Returns: (element-type utf8 Rest.Param) (transfer container): a new #GHashTable.
  **/
 GHashTable *
-rest_params_as_string_hash_table (RestParams *params)
+rest_params_as_string_hash_table (RestParams *self)
 {
-  GHashTable *hash, *strings;
-  GHashTableIter iter;
-  const char *name = NULL;
-  RestParam *param = NULL;
+  GHashTable *strings;
 
-  g_return_val_if_fail (params, NULL);
+  g_return_val_if_fail (self, NULL);
 
-  hash = (GHashTable *)params;
   strings = g_hash_table_new (g_str_hash, g_str_equal);
 
-  g_hash_table_iter_init (&iter, hash);
-  while (g_hash_table_iter_next (&iter, (gpointer)&name, (gpointer)&param)) {
-    if (rest_param_is_string (param))
-      g_hash_table_insert (strings, (gpointer)name, (gpointer)rest_param_get_content (param));
-  }
+  for (GList *cur = self->params; cur; cur = g_list_next (cur))
+    {
+      if (rest_param_is_string (cur->data))
+        g_hash_table_insert (strings, (gpointer)rest_param_get_name (cur->data), 
(gpointer)rest_param_get_content (cur->data));
+    }
 
   return strings;
 }
@@ -214,12 +268,14 @@ rest_params_as_string_hash_table (RestParams *params)
  * ]|
  **/
 void
-rest_params_iter_init (RestParamsIter *iter, RestParams *params)
+rest_params_iter_init (RestParamsIter *iter,
+                       RestParams     *params)
 {
   g_return_if_fail (iter);
   g_return_if_fail (params);
 
-  g_hash_table_iter_init ((GHashTableIter *)iter, (GHashTable *)params);
+  iter->params = params;
+  iter->position = -1;
 }
 
 /**
@@ -235,9 +291,21 @@ rest_params_iter_init (RestParamsIter *iter, RestParams *params)
  * Returns: %FALSE if the end of the #RestParams has been reached, %TRUE otherwise.
  **/
 gboolean
-rest_params_iter_next (RestParamsIter *iter, const char **name, RestParam **param)
+rest_params_iter_next (RestParamsIter  *iter,
+                       const char     **name,
+                       RestParam      **param)
 {
+  GList *cur = NULL;
+
   g_return_val_if_fail (iter, FALSE);
 
-  return g_hash_table_iter_next ((GHashTableIter *)iter, (gpointer)name, (gpointer)param);
+  iter->position++;
+  cur = g_list_nth (iter->params->params, iter->position);
+
+  if (cur == NULL) return FALSE;
+
+  *param = cur->data;
+  *name = rest_param_get_name (*param);
+  return TRUE;
 }
+
diff --git a/rest/rest-params.h b/rest/rest-params.h
index caace9d..9f53f77 100644
--- a/rest/rest-params.h
+++ b/rest/rest-params.h
@@ -20,34 +20,52 @@
  *
  */
 
-#ifndef _REST_PARAMS
-#define _REST_PARAMS
+#pragma once
 
 #include <glib-object.h>
 #include "rest-param.h"
 
 G_BEGIN_DECLS
 
-typedef struct _RestParams RestParams;
-typedef struct _GHashTableIter RestParamsIter;
-
-RestParams * rest_params_new (void);
-
-void rest_params_free (RestParams *params);
+#define REST_TYPE_PARAMS (rest_params_get_type ())
 
-void rest_params_add (RestParams *params, RestParam *param);
+typedef struct _RestParams RestParams;
+typedef struct _RestParamsIter RestParamsIter;
 
-RestParam *rest_params_get (RestParams *params, const char *name);
+struct _RestParams
+{
+  /*< private >*/
+  guint ref_count;
 
-void rest_params_remove (RestParams *params, const char *name);
+  GList *params;
+};
 
-gboolean rest_params_are_strings (RestParams *params);
+struct _RestParamsIter
+{
+  /*< private >*/
+  RestParams *params;
+  gint position;
+};
 
-GHashTable * rest_params_as_string_hash_table (RestParams *params);
+GType           rest_params_get_type (void) G_GNUC_CONST;
+RestParams *rest_params_new                  (void);
+RestParams *rest_params_copy                 (RestParams      *self);
+RestParams *rest_params_ref                  (RestParams      *self);
+void        rest_params_unref                (RestParams      *self);
+void        rest_params_add                  (RestParams      *params,
+                                              RestParam       *param);
+RestParam  *rest_params_get                  (RestParams      *params,
+                                              const char      *name);
+void        rest_params_remove               (RestParams      *params,
+                                              const char      *name);
+gboolean    rest_params_are_strings          (RestParams      *params);
+GHashTable *rest_params_as_string_hash_table (RestParams      *params);
+void        rest_params_iter_init            (RestParamsIter  *iter,
+                                              RestParams      *params);
+gboolean    rest_params_iter_next            (RestParamsIter  *iter,
+                                              const char     **name,
+                                              RestParam      **param);
 
-void rest_params_iter_init (RestParamsIter *iter, RestParams *params);
-gboolean rest_params_iter_next (RestParamsIter *iter, const char **name, RestParam **param);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC (RestParams, rest_params_unref)
 
 G_END_DECLS
-
-#endif /* _REST_PARAMS */
diff --git a/rest/rest-proxy-call.c b/rest/rest-proxy-call.c
index 62b00da..39d702f 100644
--- a/rest/rest-proxy-call.c
+++ b/rest/rest-proxy-call.c
@@ -148,7 +148,7 @@ rest_proxy_call_dispose (GObject *object)
       g_clear_object (&priv->cancellable);
     }
 
-  g_clear_pointer (&priv->params, rest_params_free);
+  g_clear_pointer (&priv->params, rest_params_unref);
   g_clear_pointer (&priv->headers, g_hash_table_unref);
   g_clear_pointer (&priv->response_headers, g_hash_table_unref);
   g_clear_object (&priv->proxy);
diff --git a/tests/meson.build b/tests/meson.build
index 0fe80e8..bddee3a 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -6,6 +6,7 @@ test_suites = {
     'oauth2',
     'xml',
     'custom-serialize',
+    'params',
   ],
   'rest-extras': [
     'flickr',
@@ -20,6 +21,11 @@ test_deps = [
   librest_extras_dep,
 ]
 
+test_env = [
+  'G_TEST_SRCDIR=@0@'.format(meson.current_source_dir()),
+  'G_TEST_BUILDDIR=@0@'.format(meson.current_build_dir()),
+]
+
 foreach suite, test_names : test_suites
   foreach name : test_names
     test_bin = executable(name,
@@ -30,6 +36,7 @@ foreach suite, test_names : test_suites
 
     test(name, test_bin,
       suite: suite,
+      env: test_env,
     )
   endforeach
 endforeach
diff --git a/tests/params.c b/tests/params.c
new file mode 100644
index 0000000..e477ba9
--- /dev/null
+++ b/tests/params.c
@@ -0,0 +1,141 @@
+#include <glib.h>
+#include "rest/rest-params.h"
+#include "rest/rest-param.h"
+#include <glib-object.h>
+
+static void
+test_params (void)
+{
+  RestParamsIter iter;
+  RestParam *param;
+  const char *name;
+  gint pos = 0;
+  g_autoptr(RestParams) params = NULL;
+
+  struct {
+    char *name;
+    char *value;
+  } data[] = {
+      {
+        .name = "name1",
+        .value = "value1"
+      },
+      {
+        .name = "name2",
+        .value = "value2"
+      }
+  };
+
+  params = rest_params_new ();
+  for (gint i = 0; i < sizeof (data)/sizeof (data[0]); i++)
+    {
+      RestParam *p = rest_param_new_string (data[i].name, REST_MEMORY_COPY, data[i].value);
+      rest_params_add (params, p);
+    }
+
+  rest_params_iter_init (&iter, params);
+  while (rest_params_iter_next (&iter, &name, &param))
+    {
+      g_assert_cmpstr (data[pos].name, ==, name);
+      g_assert_cmpstr (data[pos].value, ==, rest_param_get_content (param));
+      pos++;
+    }
+
+  rest_params_remove (params, "name2");
+  pos = 0;
+  rest_params_iter_init (&iter, params);
+  while (rest_params_iter_next (&iter, &name, &param))
+    {
+      g_assert_cmpstr (data[pos].name, ==, name);
+      g_assert_cmpstr (data[pos].value, ==, rest_param_get_content (param));
+      pos++;
+    }
+}
+
+static void
+test_params_get (void)
+{
+  g_autoptr(RestParams) params;
+  RestParam *p1;
+
+  struct {
+    char *name;
+    char *value;
+  } data[] = {
+      {
+        .name = "name1",
+        .value = "value1"
+      },
+      {
+        .name = "name2",
+        .value = "value2"
+      }
+  };
+
+  params = rest_params_new ();
+  for (gint i = 0; i < sizeof (data)/sizeof (data[0]); i++)
+    {
+      RestParam *p = rest_param_new_string (data[i].name, REST_MEMORY_COPY, data[i].value);
+      rest_params_add (params, p);
+    }
+
+  p1 = rest_params_get (params, "name2");
+
+  g_assert_cmpstr (rest_param_get_name (p1), ==, "name2");
+  g_assert_cmpstr (rest_param_get_content (p1), ==, "value2");
+}
+
+static void
+test_params_is_string (void)
+{
+  g_autoptr(GError) error = NULL;
+  g_autoptr(RestParams) params;
+  g_autofree char *file;
+  gsize length;
+  gchar *contents;
+  RestParam *p;
+
+  struct {
+    char *name;
+    char *value;
+  } data[] = {
+      {
+        .name = "name1",
+        .value = "value1"
+      },
+      {
+        .name = "name2",
+        .value = "value2"
+      }
+  };
+
+  params = rest_params_new ();
+  for (gint i = 0; i < sizeof (data)/sizeof (data[0]); i++)
+    {
+      RestParam *p = rest_param_new_string (data[i].name, REST_MEMORY_COPY, data[i].value);
+      rest_params_add (params, p);
+    }
+
+  g_assert_true (rest_params_are_strings (params));
+
+  file = g_test_build_filename (G_TEST_DIST, "test-media.png", NULL);
+  g_file_get_contents(file, &contents, &length, &error);
+
+  p = rest_param_new_full ("nostring", REST_MEMORY_COPY, contents, length, "image/png", "test-media.png");
+  rest_params_add (params, p);
+
+  g_assert_false (rest_params_are_strings (params));
+}
+
+gint
+main (gint   argc,
+      gchar *argv[])
+{
+  g_test_init (&argc, &argv, NULL);
+
+  g_test_add_func("/rest/params", test_params);
+  g_test_add_func("/rest/params_get", test_params_get);
+  g_test_add_func("/rest/params_is_strings", test_params_is_string);
+
+  return g_test_run ();
+}
diff --git a/tests/test-media.png b/tests/test-media.png
new file mode 100644
index 0000000..2dbbccc
Binary files /dev/null and b/tests/test-media.png differ