[geocode-glib/wip/issue-19] geocode-nominatim: Allow the cache path to be adjusted

[Bastien Nocera <hadess src gnome org>]

commit 443ae7e48d9cb29f4ac0020fc245823c1d1e8c7b
Author: Philip Withnall <philip withnall collabora co uk>
Date:   Thu Oct 13 16:13:17 2016 +0100

    geocode-nominatim: Allow the cache path to be adjusted
    
    Instead of always using the cache in $XDG_CACHE_HOME/geocode-glib, allow
    the path to be customised, and set it to a temporary directory while
    running the tests so we don’t accidentally use (or pollute) the user’s
    actual cache.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=756311
    
    https://bugzilla.gnome.org/show_bug.cgi?id=777196
    
    Closes: #19

 geocode-glib/geocode-glib-private.h         |  9 ++-
 geocode-glib/geocode-glib.c                 | 41 ++++++++------
 geocode-glib/geocode-nominatim.c            | 88 +++++++++++++++++++++++++++--
 geocode-glib/geocode-nominatim.h            |  4 ++
 geocode-glib/tests/geocode-glib.c           | 59 +++++++++++--------
 geocode-glib/tests/geocode-nominatim-test.c | 32 ++++++++---
 6 files changed, 177 insertions(+), 56 deletions(-)
---
diff --git a/geocode-glib/geocode-glib-private.h b/geocode-glib/geocode-glib-private.h
index 27bc66a..5d44f79 100644
--- a/geocode-glib/geocode-glib-private.h
+++ b/geocode-glib/geocode-glib-private.h
@@ -43,10 +43,13 @@ GList      *_geocode_parse_search_json  (const char *contents,
 
 char       *_geocode_object_get_lang (void);
 
-char *_geocode_glib_cache_path_for_query (SoupMessage *query);
-gboolean _geocode_glib_cache_save (SoupMessage *query,
+char *_geocode_glib_cache_path_for_query (const gchar *cache_path,
+                                          SoupMessage *query);
+gboolean _geocode_glib_cache_save (const gchar *cache_path,
+                                   SoupMessage *query,
                                    const char  *contents);
-gboolean _geocode_glib_cache_load (SoupMessage *query,
+gboolean _geocode_glib_cache_load (const gchar *cache_path,
+                                   SoupMessage *query,
                                    char       **contents);
 GHashTable *_geocode_glib_dup_hash_table (GHashTable *ht);
 gboolean _geocode_object_is_number_after_street (void);
diff --git a/geocode-glib/geocode-glib.c b/geocode-glib/geocode-glib.c
index 3b21cd2..c21066c 100644
--- a/geocode-glib/geocode-glib.c
+++ b/geocode-glib/geocode-glib.c
@@ -71,24 +71,30 @@ _geocode_glib_build_soup_session (const gchar *user_agent_override)
 }
 
 char *
-_geocode_glib_cache_path_for_query (SoupMessage *query)
+_geocode_glib_cache_path_for_query (const gchar *cache_dir,
+                                    SoupMessage *query)
 {
        const char *filename;
        char *path;
         SoupURI *soup_uri;
        char *uri;
        GChecksum *sum;
+       g_autofree gchar *allocated_cache_dir = NULL;
+
+       /* Use the default cache directory? */
+       if (cache_dir == NULL) {
+               allocated_cache_dir = g_build_filename (g_get_user_cache_dir (),
+                                                       "geocode-glib",
+                                                       NULL);
+               cache_dir = allocated_cache_dir;
+       }
 
        /* Create cache directory */
-       path = g_build_filename (g_get_user_cache_dir (),
-                                "geocode-glib",
-                                NULL);
-       if (g_mkdir_with_parents (path, 0700) < 0) {
-               g_warning ("Failed to mkdir path '%s': %s", path, g_strerror (errno));
-               g_free (path);
+       if (g_mkdir_with_parents (cache_dir, 0700) < 0) {
+               g_warning ("Failed to mkdir path '%s': %s", cache_dir,
+                          g_strerror (errno));
                return NULL;
        }
-       g_free (path);
 
        /* Create path for query */
        soup_uri = soup_message_get_uri (query);
@@ -99,10 +105,7 @@ _geocode_glib_cache_path_for_query (SoupMessage *query)
 
        filename = g_checksum_get_string (sum);
 
-       path = g_build_filename (g_get_user_cache_dir (),
-                                "geocode-glib",
-                                filename,
-                                NULL);
+       path = g_build_filename (cache_dir, filename, NULL);
 
        g_checksum_free (sum);
        g_free (uri);
@@ -110,14 +113,16 @@ _geocode_glib_cache_path_for_query (SoupMessage *query)
        return path;
 }
 
+/* @cache_path can be NULL to use the default. */
 gboolean
-_geocode_glib_cache_save (SoupMessage *query,
-                         const char  *contents)
+_geocode_glib_cache_save (const gchar *cache_path,
+                          SoupMessage *query,
+                          const char  *contents)
 {
        char *path;
        gboolean ret;
 
-       path = _geocode_glib_cache_path_for_query (query);
+       path = _geocode_glib_cache_path_for_query (cache_path, query);
        g_debug ("Saving cache file '%s'", path);
        ret = g_file_set_contents (path, contents, -1, NULL);
 
@@ -125,14 +130,16 @@ _geocode_glib_cache_save (SoupMessage *query,
        return ret;
 }
 
+/* @cache_path can be NULL to use the default. */
 gboolean
-_geocode_glib_cache_load (SoupMessage *query,
+_geocode_glib_cache_load (const gchar *cache_path,
+                         SoupMessage *query,
                          char  **contents)
 {
        char *path;
        gboolean ret;
 
-       path = _geocode_glib_cache_path_for_query (query);
+       path = _geocode_glib_cache_path_for_query (cache_path, query);
        g_debug ("Loading cache file '%s'", path);
        ret = g_file_get_contents (path, contents, NULL, NULL);
 
diff --git a/geocode-glib/geocode-nominatim.c b/geocode-glib/geocode-nominatim.c
index fc333dc..251200b 100644
--- a/geocode-glib/geocode-nominatim.c
+++ b/geocode-glib/geocode-nominatim.c
@@ -50,14 +50,16 @@ typedef enum {
        PROP_BASE_URL = 1,
        PROP_MAINTAINER_EMAIL_ADDRESS,
        PROP_USER_AGENT,
+       PROP_CACHE_PATH,
 } GeocodeNominatimProperty;
 
-static GParamSpec *properties[PROP_USER_AGENT + 1];
+static GParamSpec *properties[PROP_CACHE_PATH + 1];
 
 typedef struct {
        char *base_url;
        char *maintainer_email_address;
        char *user_agent;
+       char *cache_path;
 } GeocodeNominatimPrivate;
 
 static void geocode_backend_iface_init (GeocodeBackendInterface *iface);
@@ -880,8 +882,13 @@ on_query_data_loaded (SoupSession *session,
                       SoupMessage *query,
                       GTask       *task)
 {
+       GeocodeNominatim *self;
+       GeocodeNominatimPrivate *priv;
        char *contents;
 
+       self = g_task_get_source_object (task);
+       priv = geocode_nominatim_get_instance_private (self);
+
        if (query->status_code != SOUP_STATUS_OK)
                g_task_return_new_error (task,
                                         G_IO_ERROR,
@@ -890,7 +897,7 @@ on_query_data_loaded (SoupSession *session,
                                         query->reason_phrase ? query->reason_phrase : "Query failed");
        else {
                contents = g_strndup (query->response_body->data, query->response_body->length);
-               _geocode_glib_cache_save (query, contents);
+               _geocode_glib_cache_save (priv->cache_path, query, contents);
                g_task_return_pointer (task, contents, g_free);
        }
 
@@ -951,7 +958,8 @@ geocode_nominatim_query_async (GeocodeNominatim    *self,
        soup_query = soup_message_new (SOUP_METHOD_GET, uri);
        g_task_set_task_data (task, soup_query, g_object_unref);
 
-       cache_path = _geocode_glib_cache_path_for_query (soup_query);
+       cache_path = _geocode_glib_cache_path_for_query (priv->cache_path,
+                                                        soup_query);
        if (cache_path != NULL) {
                GFile *cache;
 
@@ -994,14 +1002,14 @@ geocode_nominatim_query (GeocodeNominatim  *self,
        soup_session = _geocode_glib_build_soup_session (priv->user_agent);
        soup_query = soup_message_new (SOUP_METHOD_GET, uri);
 
-       if (_geocode_glib_cache_load (soup_query, &contents) == FALSE) {
+       if (_geocode_glib_cache_load (priv->cache_path, soup_query, &contents) == FALSE) {
                if (soup_session_send_message (soup_session, soup_query) != SOUP_STATUS_OK) {
                        g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED,
                                             soup_query->reason_phrase ? soup_query->reason_phrase : "Query 
failed");
                        contents = NULL;
                } else {
                        contents = g_strndup (soup_query->response_body->data, 
soup_query->response_body->length);
-                       _geocode_glib_cache_save (soup_query, contents);
+                       _geocode_glib_cache_save (priv->cache_path, soup_query, contents);
                }
        }
 
@@ -1355,6 +1363,53 @@ geocode_nominatim_new (const char *base_url,
                                                NULL));
 }
 
+/**
+ * geocode_nominatim_get_cache_path:
+ * @self: a #GeocodeNominatim
+ *
+ * Get the current value of the cache path where query results are cached
+ * locally to avoid excess network round trips. This is the
+ * #GeocodeNominatim:cache-path property.
+ *
+ * Returns: (transfer none) (nullable): the current cache path, or %NULL if the
+ *    default is in use
+ * Since: UNRELEASED
+ */
+const gchar *
+geocode_nominatim_get_cache_path (GeocodeNominatim *self)
+{
+       GeocodeNominatimPrivate *priv = geocode_nominatim_get_instance_private (self);
+
+       g_return_val_if_fail (GEOCODE_IS_NOMINATIM (self), NULL);
+
+       return priv->cache_path;
+}
+
+/**
+ * geocode_nominatim_set_cache_path:
+ * @self: a #GeocodeNominatim
+ * @path: (nullable): new cache path to use, or %NULL to use the default
+ *
+ * Set the path where query results are cached, #GeocodeNominatim:cache-path.
+ *
+ * Since: UNRELEASED
+ */
+void
+geocode_nominatim_set_cache_path (GeocodeNominatim *self,
+                                  const gchar      *path)
+{
+       GeocodeNominatimPrivate *priv = geocode_nominatim_get_instance_private (self);
+
+       g_return_if_fail (GEOCODE_IS_NOMINATIM (self));
+
+       if (g_strcmp0 (path, priv->cache_path) == 0)
+               return;
+
+       g_free (priv->cache_path);
+       priv->cache_path = g_strdup (path);
+       g_object_notify (G_OBJECT (self), "cache-path");
+}
+
 static void
 geocode_nominatim_init (GeocodeNominatim *object)
 {
@@ -1395,6 +1450,9 @@ geocode_nominatim_get_property (GObject    *object,
        case PROP_USER_AGENT:
                g_value_set_string (value, priv->user_agent);
                break;
+       case PROP_CACHE_PATH:
+               g_value_set_string (value, priv->cache_path);
+               break;
        default:
                /* We don't have any other property... */
                G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec);
@@ -1431,6 +1489,10 @@ geocode_nominatim_set_property (GObject      *object,
                                                  properties[PROP_USER_AGENT]);
                }
                break;
+       case PROP_CACHE_PATH:
+               geocode_nominatim_set_cache_path (GEOCODE_NOMINATIM (object),
+                                                 g_value_get_string (value));
+               break;
        default:
                /* We don't have any other property... */
                G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec);
@@ -1448,6 +1510,7 @@ geocode_nominatim_finalize (GObject *object)
        g_free (priv->base_url);
        g_free (priv->maintainer_email_address);
        g_free (priv->user_agent);
+       g_free (priv->cache_path);
 
        G_OBJECT_CLASS (geocode_nominatim_parent_class)->finalize (object);
 }
@@ -1536,6 +1599,21 @@ geocode_nominatim_class_init (GeocodeNominatimClass *klass)
                                                           (G_PARAM_READWRITE |
                                                            G_PARAM_STATIC_STRINGS));
 
+       /**
+        * GeocodeNominatim:cache-path:
+        *
+        * Path to the query results cache on the file system, or %NULL to use
+        * the default cache path (`$XDG_CACHE_HOME/geocode-glib`).
+        *
+        * Since: UNRELEASED
+        */
+       properties[PROP_CACHE_PATH] = g_param_spec_string ("cache-path",
+                                                          "Cache path",
+                                                          "Path to the query results cache",
+                                                          NULL,
+                                                          (G_PARAM_READWRITE |
+                                                           G_PARAM_STATIC_STRINGS));
+
        g_object_class_install_properties (object_class,
                                           G_N_ELEMENTS (properties), properties);
 }
diff --git a/geocode-glib/geocode-nominatim.h b/geocode-glib/geocode-nominatim.h
index a2b3926..70008ff 100644
--- a/geocode-glib/geocode-nominatim.h
+++ b/geocode-glib/geocode-nominatim.h
@@ -89,6 +89,10 @@ struct _GeocodeNominatimClass {
 GeocodeNominatim *geocode_nominatim_new (const gchar *base_url,
                                          const gchar *maintainer_email_address);
 
+const gchar *geocode_nominatim_get_cache_path (GeocodeNominatim *self);
+void         geocode_nominatim_set_cache_path (GeocodeNominatim *self,
+                                               const gchar      *path);
+
 GeocodeNominatim *geocode_nominatim_get_gnome (void);
 
 G_END_DECLS
diff --git a/geocode-glib/tests/geocode-glib.c b/geocode-glib/tests/geocode-glib.c
index e7d6d84..06a5148 100644
--- a/geocode-glib/tests/geocode-glib.c
+++ b/geocode-glib/tests/geocode-glib.c
@@ -174,28 +174,33 @@ load_json (const gchar *expected_response_filename)
 }
 
 static void
-set_up_cache (void)
+backend_set_up_cache (GeocodeBackend *backend)
 {
-       g_autofree gchar *cache_path = NULL;
-       g_autoptr (GError) error = NULL;
-
-       cache_path = g_dir_make_tmp ("test-gcglib-XXXXXX", &error);
-       g_assert_no_error (error);
-
-       g_setenv ("XDG_CACHE_HOME", cache_path, TRUE);
+       /* If the network is not enabled, we are using a #GeocodeNominatimTest
+        * backend, which handles its own temporary cache directory. */
+       if (enable_network && GEOCODE_IS_NOMINATIM (backend)) {
+               g_autofree gchar *cache_path = NULL;
+               g_autoptr (GError) error = NULL;
+
+               cache_path = g_dir_make_tmp ("test-gcglib-XXXXXX", &error);
+               g_assert_no_error (error);
+               geocode_nominatim_set_cache_path (GEOCODE_NOMINATIM (backend),
+                                                 cache_path);
+       }
 }
 
 static GeocodeReverse *
 create_reverse (GeocodeLocation *loc,
                 const gchar     *expected_response_filename)
 {
+       g_autoptr (GeocodeBackend) backend = NULL;
        g_autoptr (GHashTable) parameters = NULL;
        g_autoptr (GeocodeReverse) reverse = NULL;
        char lat[G_ASCII_DTOSTR_BUF_SIZE];
        char lon[G_ASCII_DTOSTR_BUF_SIZE];
 
        /* Set up the cache to avoid polluting the user’s main cache. */
-       set_up_cache ();
+       backend_set_up_cache (backend);
 
        /* Build the query parameters. */
        g_ascii_dtostr (lat,
@@ -213,20 +218,22 @@ create_reverse (GeocodeLocation *loc,
        reverse = geocode_reverse_new_for_location (loc);
 
        if (!enable_network) {
-               g_autoptr (GeocodeNominatim) backend = NULL;
                g_autofree gchar *expected_response = NULL;
 
                /* Load the JSON we expect as a response. */
                expected_response = load_json (expected_response_filename);
 
                /* Build the backend and query object. */
-               backend = geocode_nominatim_test_new ();
+               backend = GEOCODE_BACKEND (geocode_nominatim_test_new ());
                geocode_nominatim_test_expect_query (GEOCODE_NOMINATIM_TEST (backend),
                                                     parameters, expected_response);
-
-               geocode_reverse_set_backend (reverse, GEOCODE_BACKEND (backend));
+       } else {
+               backend = GEOCODE_BACKEND (geocode_nominatim_get_gnome ());
        }
 
+       backend_set_up_cache (backend);
+       geocode_reverse_set_backend (reverse, backend);
+
        return g_steal_pointer (&reverse);
 }
 
@@ -242,26 +249,29 @@ create_forward_for_params (GHashTable  *tp,
                            GHashTable  *params,
                            const gchar *expected_response_filename)
 {
+       g_autoptr (GeocodeBackend) backend = NULL;
        g_autoptr (GeocodeForward) forward = NULL;
 
        /* Set up the cache to avoid polluting the user’s main cache. */
-       set_up_cache ();
+       backend_set_up_cache (backend);
 
        forward = geocode_forward_new_for_params (tp);
 
        if (!enable_network) {
-               g_autoptr (GeocodeNominatim) backend = NULL;
                g_autofree gchar *expected_response = NULL;
 
                expected_response = load_json (expected_response_filename);
 
-               backend = geocode_nominatim_test_new ();
+               backend = GEOCODE_BACKEND (geocode_nominatim_test_new ());
                geocode_nominatim_test_expect_query (GEOCODE_NOMINATIM_TEST (backend),
                                                     params, expected_response);
-
-               geocode_forward_set_backend (forward, GEOCODE_BACKEND (backend));
+       } else {
+               backend = GEOCODE_BACKEND (geocode_nominatim_get_gnome ());
        }
 
+       backend_set_up_cache (backend);
+       geocode_forward_set_backend (forward, backend);
+
        return g_steal_pointer (&forward);
 }
 
@@ -277,26 +287,29 @@ create_forward_for_string (const gchar *q,
                            GHashTable  *params,
                            const gchar *expected_response_filename)
 {
+       g_autoptr (GeocodeBackend) backend = NULL;
        g_autoptr (GeocodeForward) forward = NULL;
 
        /* Set up the cache to avoid polluting the user’s main cache. */
-       set_up_cache ();
+       backend_set_up_cache (backend);
 
        forward = geocode_forward_new_for_string (q);
 
        if (!enable_network) {
-               g_autoptr (GeocodeNominatim) backend = NULL;
                g_autofree gchar *expected_response = NULL;
 
                expected_response = load_json (expected_response_filename);
 
-               backend = geocode_nominatim_test_new ();
+               backend = GEOCODE_BACKEND (geocode_nominatim_test_new ());
                geocode_nominatim_test_expect_query (GEOCODE_NOMINATIM_TEST (backend),
                                                     params, expected_response);
-
-               geocode_forward_set_backend (forward, GEOCODE_BACKEND (backend));
+       } else {
+               backend = GEOCODE_BACKEND (geocode_nominatim_get_gnome ());
        }
 
+       backend_set_up_cache (backend);
+       geocode_forward_set_backend (forward, backend);
+
        return g_steal_pointer (&forward);
 }
 
diff --git a/geocode-glib/tests/geocode-nominatim-test.c b/geocode-glib/tests/geocode-nominatim-test.c
index d185ff1..da03273 100644
--- a/geocode-glib/tests/geocode-nominatim-test.c
+++ b/geocode-glib/tests/geocode-nominatim-test.c
@@ -209,17 +209,33 @@ real_query (GeocodeNominatim  *self,
 
 /******************************************************************************/
 
+/**
+ * geocode_nominatim_test_new:
+ *
+ * Create a new #GeocodeNominatimTest instance, set up with a dummy base URI
+ * and maintainer e-mail address, and with its cache directory set to a new,
+ * empty temporary directory.
+ *
+ * Returns: (transfer full): a new #GeocodeNominatimTest
+ * Since: UNRELEASED
+ */
 GeocodeNominatim *
 geocode_nominatim_test_new (void)
 {
-       /* This shouldn’t be used with the user’s normal cache directory, or we
-        * will pollute it. */
-       g_assert (g_str_has_prefix (g_get_user_cache_dir (), g_get_tmp_dir ()));
-
-       return GEOCODE_NOMINATIM (g_object_new (GEOCODE_TYPE_NOMINATIM_TEST,
-                                               "base-url", "http://example.invalid";,
-                                               "maintainer-email-address", "maintainer@invalid",
-                                               NULL));
+       g_autoptr (GeocodeNominatim) nominatim = NULL;
+       g_autofree gchar *cache_path = NULL;
+       g_autoptr (GError) error = NULL;
+
+       nominatim = GEOCODE_NOMINATIM (g_object_new (GEOCODE_TYPE_NOMINATIM_TEST,
+                                                    "base-url", "http://example.invalid";,
+                                                    "maintainer-email-address", "maintainer@invalid",
+                                                    NULL));
+
+       cache_path = g_dir_make_tmp ("geocode-nominatim-test-XXXXXX", &error);
+       g_assert_no_error (error);
+       geocode_nominatim_set_cache_path (nominatim, cache_path);
+
+       return g_steal_pointer (&nominatim);
 }
 
 static void