[libshumate] file-cache: Return tile modification time

[Corentin Noël <corentinnoel src gnome org>]

commit f2eaeee92fe347b6626c181339efb6f957318cea
Author: James Westman <james jwestman net>
Date:   Wed Apr 7 00:02:34 2021 -0500

    file-cache: Return tile modification time
    
    Change get_tile_finish() to provide the modification time of a tile.
    This allows the network tile source to use it in requests for caching,
    and makes expiration checking more robust since it doesn't rely on the
    presence of an ETag.

 shumate/shumate-file-cache.c          | 50 +++++++++++++--------
 shumate/shumate-file-cache.h          |  1 +
 shumate/shumate-network-tile-source.c | 21 ++++++---
 tests/file-cache.c                    | 85 ++++++-----------------------------
 4 files changed, 62 insertions(+), 95 deletions(-)
---
diff --git a/shumate/shumate-file-cache.c b/shumate/shumate-file-cache.c
index 3f1615d..e9a6990 100644
--- a/shumate/shumate-file-cache.c
+++ b/shumate/shumate-file-cache.c
@@ -487,16 +487,6 @@ get_filename (ShumateFileCache *file_cache,
 }
 
 
-static gboolean
-tile_is_expired (ShumateFileCache *file_cache, GDateTime *modified_time)
-{
-  g_autoptr(GDateTime) now = g_date_time_new_now_utc ();
-  GTimeSpan diff = g_date_time_difference (now, modified_time);
-
-  return diff > 7 * G_TIME_SPAN_DAY; /* Cache expires in 7 days */
-}
-
-
 static char *
 db_get_etag (ShumateFileCache *self, ShumateTile *tile)
 {
@@ -744,6 +734,18 @@ shumate_file_cache_purge (ShumateFileCache *file_cache)
   sqlite3_exec (priv->db, "PRAGMA incremental_vacuum;", NULL, NULL, &error);
 }
 
+typedef struct {
+  char *etag;
+  GDateTime *modtime;
+} GetTileData;
+
+static void
+get_tile_data_free (GetTileData *data)
+{
+  g_clear_pointer (&data->etag, g_free);
+  g_clear_pointer (&data->modtime, g_date_time_unref);
+  g_free (data);
+}
 
 static void on_get_tile_file_loaded (GObject *source_object, GAsyncResult *res, gpointer user_data);
 
@@ -770,7 +772,7 @@ shumate_file_cache_get_tile_async (ShumateFileCache *self,
   g_autofree char *filename = NULL;
   g_autoptr(GFileInfo) info = NULL;
   g_autoptr(GError) error = NULL;
-  g_autoptr(GDateTime) modified_time = NULL;
+  GetTileData *task_data = NULL;
 
   g_return_if_fail (SHUMATE_IS_FILE_CACHE (self));
   g_return_if_fail (SHUMATE_IS_TILE (tile));
@@ -779,6 +781,9 @@ shumate_file_cache_get_tile_async (ShumateFileCache *self,
   task = g_task_new (self, cancellable, callback, user_data);
   g_task_set_source_tag (task, shumate_file_cache_get_tile_async);
 
+  task_data = g_new0 (GetTileData, 1);
+  g_task_set_task_data (task, task_data, (GDestroyNotify) get_tile_data_free);
+
   filename = get_filename (self, tile);
   file = g_file_new_for_path (filename);
 
@@ -796,12 +801,10 @@ shumate_file_cache_get_tile_async (ShumateFileCache *self,
       return;
     }
 
-  modified_time = g_file_info_get_modification_date_time (info);
-  shumate_tile_set_modified_time (tile, modified_time);
+  task_data->modtime = g_file_info_get_modification_date_time (info);
+  shumate_tile_set_modified_time (tile, task_data->modtime);
 
-  /* If the tile is expired, set the ETag */
-  if (tile_is_expired (self, modified_time))
-    g_task_set_task_data (task, db_get_etag (self, tile), g_free);
+  task_data->etag = db_get_etag (self, tile);
 
   /* update tile popularity */
   on_tile_filled (self, tile);
@@ -838,14 +841,18 @@ on_get_tile_file_loaded (GObject *source_object, GAsyncResult *res, gpointer use
 /**
  * shumate_file_cache_get_tile_finish:
  * @self: a #ShumateFileCache
- * @etag: a location for the data's ETag, or %NULL
+ * @etag: (nullable) (out) (optional): a location for the data's ETag, or %NULL
+ * @modtime: (nullable) (out) (optional): a location to return the tile's last modification time, or %NULL
  * @result: a #GAsyncResult provided to callback
  * @error: a location for a #GError, or %NULL
  *
  * Gets the tile data from a completed shumate_file_cache_get_tile_async()
  * operation.
  *
- * @etag will only be set if the tile data is potentially out of date.
+ * @modtime will be set to the time the tile was added to the cache, or the
+ * latest time it was confirmed to be up to date.
+ *
+ * @etag will be set to the data's ETag, if present.
  *
  * Returns: a #GBytes containing the tile data, or %NULL if the tile was not in
  * the cache or an error occurred
@@ -853,14 +860,19 @@ on_get_tile_file_loaded (GObject *source_object, GAsyncResult *res, gpointer use
 GBytes *
 shumate_file_cache_get_tile_finish (ShumateFileCache *self,
                                     char **etag,
+                                    GDateTime **modtime,
                                     GAsyncResult *result,
                                     GError **error)
 {
+  GetTileData *data = g_task_get_task_data (G_TASK (result));
+
   g_return_val_if_fail (SHUMATE_IS_FILE_CACHE (self), NULL);
   g_return_val_if_fail (g_task_is_valid (result, self), NULL);
 
   if (etag)
-    *etag = g_strdup (g_task_get_task_data (G_TASK (result)));
+    *etag = g_steal_pointer (&data->etag);
+  if (modtime)
+    *modtime = g_steal_pointer (&data->modtime);
 
   return g_task_propagate_pointer (G_TASK (result), error);
 }
diff --git a/shumate/shumate-file-cache.h b/shumate/shumate-file-cache.h
index 8c382f5..7b46ee2 100644
--- a/shumate/shumate-file-cache.h
+++ b/shumate/shumate-file-cache.h
@@ -86,6 +86,7 @@ void shumate_file_cache_get_tile_async (ShumateFileCache *self,
                                         gpointer user_data);
 GBytes *shumate_file_cache_get_tile_finish (ShumateFileCache *self,
                                             char **etag,
+                                            GDateTime **modtime,
                                             GAsyncResult *result,
                                             GError **error);
 
diff --git a/shumate/shumate-network-tile-source.c b/shumate/shumate-network-tile-source.c
index 3637bd4..36956d8 100644
--- a/shumate/shumate-network-tile-source.c
+++ b/shumate/shumate-network-tile-source.c
@@ -668,6 +668,16 @@ get_tile_uri (ShumateNetworkTileSource *tile_source,
 }
 
 
+static gboolean
+tile_is_expired (GDateTime *modified_time)
+{
+  g_autoptr(GDateTime) now = g_date_time_new_now_utc ();
+  GTimeSpan diff = g_date_time_difference (now, modified_time);
+
+  return diff > 7 * G_TIME_SPAN_DAY; /* Cache expires in 7 days */
+}
+
+
 static void
 on_pixbuf_created (GObject      *source_object,
                    GAsyncResult *res,
@@ -847,17 +857,18 @@ on_file_cache_get_tile (GObject *source_object, GAsyncResult *res, gpointer user
   FillTileData *data = g_task_get_task_data (task);
   GCancellable *cancellable = g_task_get_cancellable (task);
   ShumateNetworkTileSourcePrivate *priv = shumate_network_tile_source_get_instance_private (data->self);
+  g_autoptr(GDateTime) modtime = NULL;
   g_autofree char *uri = NULL;
   g_autofree char *etag = NULL;
   g_autoptr(GBytes) bytes = NULL;
 
-  bytes = shumate_file_cache_get_tile_finish (SHUMATE_FILE_CACHE (source_object), &etag, res, NULL);
+  bytes = shumate_file_cache_get_tile_finish (SHUMATE_FILE_CACHE (source_object),
+                                              &etag, &modtime, res, NULL);
 
-  if (bytes && etag == NULL)
+  if (bytes && !tile_is_expired (modtime))
     {
-      /* No need to fetch new data from the network (the file cache does not
-       * set the etag when the data is up to date). Just fill the tile directly
-       * from the cache. */
+      /* No need to fetch new data from the network. Just fill the tile
+       * directly from the cache. */
 
       g_autoptr(GInputStream) input_stream = g_memory_input_stream_new_from_bytes (bytes);
       gdk_pixbuf_new_from_stream_async (input_stream, cancellable, on_pixbuf_created_from_cache, 
g_object_ref (task));
diff --git a/tests/file-cache.c b/tests/file-cache.c
index 238d484..0d4f1d2 100644
--- a/tests/file-cache.c
+++ b/tests/file-cache.c
@@ -18,21 +18,23 @@ on_tile_stored (GObject *object, GAsyncResult *res, gpointer user_data)
 }
 
 static void
-on_tile_retrieved_no_etag (GObject *object, GAsyncResult *res, gpointer user_data)
+on_tile_retrieved (GObject *object, GAsyncResult *res, gpointer user_data)
 {
-  g_autoptr(GError) error = NULL;
   GMainLoop *loop = user_data;
-  g_autofree char *etag = NULL;
+  g_autoptr(GError) error = NULL;
   g_autoptr(GBytes) bytes = NULL;
+  g_autofree char *etag = NULL;
   g_autoptr(GBytes) expected_bytes = g_bytes_new_static (TEST_DATA, sizeof TEST_DATA);
+  g_autoptr(GDateTime) modtime = NULL;
+  g_autoptr(GDateTime) now = g_date_time_new_now_utc ();
 
-  /* Make sure the tile is there and the data is correct */
-  bytes = shumate_file_cache_get_tile_finish ((ShumateFileCache *) object, &etag, res, &error);
+  bytes = shumate_file_cache_get_tile_finish ((ShumateFileCache *) object, &etag, &modtime, res, &error);
   g_assert_no_error (error);
-  g_assert_true (g_bytes_equal (bytes, expected_bytes));
 
-  /* There should be no etag because the tile is recent */
-  g_assert_null (etag);
+  g_assert_true (g_bytes_equal (bytes, expected_bytes));
+  g_assert_cmpstr (etag, ==, TEST_ETAG);
+  /* the modification time should be very, very recent */
+  g_assert_true (g_date_time_difference (now, modtime) < G_TIME_SPAN_SECOND * 10);
 
   g_main_loop_quit (loop);
 }
@@ -56,7 +58,7 @@ test_file_cache_store_retrieve ()
   /* Now retrieve it */
   g_main_loop_unref (loop);
   loop = g_main_loop_new (NULL, TRUE);
-  shumate_file_cache_get_tile_async (cache, tile, NULL, on_tile_retrieved_no_etag, loop);
+  shumate_file_cache_get_tile_async (cache, tile, NULL, on_tile_retrieved, loop);
   g_main_loop_run (loop);
 }
 
@@ -68,12 +70,14 @@ on_no_tile_retrieved (GObject *object, GAsyncResult *res, gpointer user_data)
   g_autoptr(GError) error = NULL;
   g_autoptr(GBytes) bytes = NULL;
   g_autofree char *etag = NULL;
+  g_autoptr(GDateTime) modtime = NULL;
 
   /* Make sure retrieving the tile returns NULL */
-  bytes = shumate_file_cache_get_tile_finish ((ShumateFileCache *) object, &etag, res, &error);
+  bytes = shumate_file_cache_get_tile_finish ((ShumateFileCache *) object, &etag, &modtime, res, &error);
   g_assert_no_error (error);
   g_assert_null (bytes);
   g_assert_null (etag);
+  g_assert_null (modtime);
 
   g_main_loop_quit (loop);
 }
@@ -94,66 +98,6 @@ test_file_cache_miss ()
 }
 
 
-
-static void
-on_tile_retrieved_with_etag (GObject *object, GAsyncResult *res, gpointer user_data)
-{
-  g_autoptr(GError) error = NULL;
-  GMainLoop *loop = user_data;
-  g_autofree char *etag = NULL;
-  g_autoptr(GBytes) bytes = NULL;
-  g_autoptr(GBytes) expected_bytes = g_bytes_new_static (TEST_DATA, sizeof TEST_DATA);
-
-  /* Make sure the tile is there and the data is correct */
-  bytes = shumate_file_cache_get_tile_finish ((ShumateFileCache *) object, &etag, res, &error);
-  g_assert_no_error (error);
-  g_assert_true (g_bytes_equal (bytes, expected_bytes));
-  g_assert_cmpstr (etag, ==, TEST_ETAG);
-
-  g_main_loop_quit (loop);
-}
-
-/* Test that potentially out-of-date tile data returns an ETag */
-static void
-test_file_cache_etag ()
-{
-  g_autoptr(ShumateFileCache) cache = shumate_file_cache_new_full (100000000, "test", NULL);
-  g_autoptr(ShumateTile) tile = shumate_tile_new_full (0, 0, 256, 0);
-  g_autoptr(GBytes) bytes = g_bytes_new_static (TEST_DATA, sizeof TEST_DATA);
-  g_autoptr(GMainLoop) loop = NULL;
-  g_autoptr(GFile) file = NULL;
-  guint64 mod_time = 1000;
-
-  g_object_ref_sink (tile);
-
-  /* Store the tile */
-  loop = g_main_loop_new (NULL, TRUE);
-  shumate_file_cache_store_tile_async (cache, tile, bytes, TEST_ETAG, NULL, on_tile_stored, loop);
-  g_main_loop_run (loop);
-
-  /* Change the modified date of the file */
-  file = g_file_new_build_filename (g_get_user_cache_dir(), "shumate/test/0/0/0.png", NULL);
-  g_file_set_attribute (file, G_FILE_ATTRIBUTE_TIME_MODIFIED,
-                        G_FILE_ATTRIBUTE_TYPE_UINT64, &mod_time,
-                        G_FILE_QUERY_INFO_NONE, NULL, NULL);
-
-  /* Now retrieve it */
-  g_main_loop_unref (loop);
-  loop = g_main_loop_new (NULL, TRUE);
-  shumate_file_cache_get_tile_async (cache, tile, NULL, on_tile_retrieved_with_etag, loop);
-  g_main_loop_run (loop);
-
-  /* Mark the tile up to date */
-  shumate_file_cache_mark_up_to_date (cache, tile);
-
-  /* There should be no ETag this time */
-  g_main_loop_unref (loop);
-  loop = g_main_loop_new (NULL, TRUE);
-  shumate_file_cache_get_tile_async (cache, tile, NULL, on_tile_retrieved_no_etag, loop);
-  g_main_loop_run (loop);
-}
-
-
 int
 main (int argc, char *argv[])
 {
@@ -162,7 +106,6 @@ main (int argc, char *argv[])
 
   g_test_add_func ("/file-cache/store-retrieve", test_file_cache_store_retrieve);
   g_test_add_func ("/file-cache/miss", test_file_cache_miss);
-  g_test_add_func ("/file-cache/etag", test_file_cache_etag);
 
   return g_test_run ();
 }