geocode-nominatim: Allow the cache path to be adjustedwip/issue-19

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

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