[gnome-software/wip/jrocha/fix-screenshots] Fix stale screenshots when they are cached
- From: Joaquim Manuel Pereira Rocha <jrocha src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gnome-software/wip/jrocha/fix-screenshots] Fix stale screenshots when they are cached
- Date: Tue, 8 Nov 2016 16:37:50 +0000 (UTC)
commit 746d822601e1ec1d435d99cd686038be0871ca64
Author: Joaquim Rocha <jrocha endlessm com>
Date: Tue Nov 8 17:00:04 2016 +0100
Fix stale screenshots when they are cached
This patch solves a couple of issues in the way we use the screenshots'
cache:
1) Once a screenshot was downloaded, it was never updated again until
there was a new version of GNOME Software that would initialize a
new cache. With these changes we always check the screenshots
remotely so we update the files as soon as there are newer ones on
the server. However, we use the images' file modification when
checking the remote files so we only effectively download those if
they are newer.
2) If screenshots were present in the system's cache, then they would
not be updated even on a new version of GNOME Software and they
would be used even if the user cache's version was newer.
3) It was downloading an image for every screenshot shown; this would
end up in unnecessary downloads when the there are not different
images for thumbnails/large screenshots. This patch saves a the
counterpart sizes when downloading a thumbnail/large image if their
kind is unknown (which means that when the image have an assigned
'thumbnail' or 'source' kind, they still get downloaded
individually).
src/gs-screenshot-image.c | 185 +++++++++++++++++++++++++++++++++++++++------
src/gs-utils.c | 27 +++++++
src/gs-utils.h | 3 +
3 files changed, 192 insertions(+), 23 deletions(-)
---
diff --git a/src/gs-screenshot-image.c b/src/gs-screenshot-image.c
index 01fbcf4..0644aa9 100644
--- a/src/gs-screenshot-image.c
+++ b/src/gs-screenshot-image.c
@@ -35,6 +35,13 @@
#include "gs-common.h"
#include "gs-utils.h"
+typedef enum {
+ GS_SCREENSHOT_IMAGE_STATUS_UNKNOWN,
+ GS_SCREENSHOT_IMAGE_STATUS_IMAGE,
+ GS_SCREENSHOT_IMAGE_STATUS_BLURRED,
+ GS_SCREENSHOT_IMAGE_STATUS_ERROR
+} GsScreenshotImageStatus;
+
struct _GsScreenshotImage
{
GtkBin parent_instance;
@@ -53,10 +60,13 @@ struct _GsScreenshotImage
guint width;
guint height;
guint scale;
+ GsScreenshotImageStatus status;
};
G_DEFINE_TYPE (GsScreenshotImage, gs_screenshot_image, GTK_TYPE_BIN)
+static gchar *gs_screenshot_get_cachefn_for_url (const gchar *url);
+
AsScreenshot *
gs_screenshot_image_get_screenshot (GsScreenshotImage *ssimg)
{
@@ -76,6 +86,8 @@ gs_screenshot_image_set_error (GsScreenshotImage *ssimg, const gchar *message)
gtk_widget_hide (ssimg->label_error);
else
gtk_widget_show (ssimg->label_error);
+
+ ssimg->status = GS_SCREENSHOT_IMAGE_STATUS_ERROR;
}
static GdkPixbuf *
@@ -168,6 +180,7 @@ as_screenshot_show_image (GsScreenshotImage *ssimg)
ssimg->current_image = "image1";
}
+ ssimg->status = GS_SCREENSHOT_IMAGE_STATUS_IMAGE;
gtk_widget_show (GTK_WIDGET (ssimg));
}
@@ -196,6 +209,59 @@ gs_screenshot_image_show_blurred (GsScreenshotImage *ssimg,
gs_image_set_from_pixbuf_with_scale (GTK_IMAGE (ssimg->image2),
pb, (gint) ssimg->scale);
}
+
+ ssimg->status = GS_SCREENSHOT_IMAGE_STATUS_BLURRED;
+}
+
+static void
+gs_screenshot_image_save_counterpart (GsScreenshotImage *ssimg, AsImage *im)
+{
+ guint width = ssimg->width;
+ guint height = ssimg->height;
+ gboolean ret;
+ g_autoptr(GError) error = NULL;
+ const char *url = as_image_get_url (im);
+ g_autofree char *filename = NULL;
+ g_autofree char *size_dir = NULL;
+ g_autofree char *cache_kind = NULL;
+ g_autofree char *basename = gs_screenshot_get_cachefn_for_url (url);
+
+ /* if the image is a thumbnail we save the normal size and
+ * vice-versa */
+ if (width == AS_IMAGE_THUMBNAIL_WIDTH &&
+ height == AS_IMAGE_THUMBNAIL_HEIGHT) {
+ width = AS_IMAGE_NORMAL_WIDTH;
+ height = AS_IMAGE_NORMAL_HEIGHT;
+ } else {
+ width = AS_IMAGE_THUMBNAIL_WIDTH;
+ height = AS_IMAGE_THUMBNAIL_HEIGHT;
+ }
+
+ size_dir = g_strdup_printf ("%ux%u", width * ssimg->scale,
+ height * ssimg->scale);
+ cache_kind = g_build_filename ("screenshots", size_dir, NULL);
+ filename = gs_utils_get_cache_filename (cache_kind,
+ basename,
+ GS_UTILS_CACHE_FLAG_WRITEABLE,
+ NULL);
+
+ if (!filename) {
+ g_warning ("Failed to get cache filename for counterpart "
+ "screenshot for URL '%s' in folder '%s'", url,
+ cache_kind);
+ return;
+ }
+
+ ret = as_image_save_filename (im, filename,
+ width * ssimg->scale,
+ height * ssimg->scale,
+ AS_IMAGE_SAVE_FLAG_PAD_16_9,
+ &error);
+
+ if (!ret)
+ g_warning ("Failed to save counterpart screenshot for URL '%s' "
+ "in folder '%s': %s", url, cache_kind,
+ error->message);
}
static void
@@ -214,7 +280,26 @@ gs_screenshot_image_complete_cb (SoupSession *session,
if (msg->status_code == SOUP_STATUS_CANCELLED || ssimg->session == NULL)
return;
+ if (msg->status_code == SOUP_STATUS_NOT_MODIFIED) {
+ SoupURI *uri = soup_message_get_uri (msg);
+ g_autofree char *url = soup_uri_to_string (uri, FALSE);
+
+ g_debug ("Screenshot for '%s' has not been modified; not "
+ "downloading again", url);
+ return;
+ }
if (msg->status_code != SOUP_STATUS_OK) {
+ SoupURI *uri = soup_message_get_uri (msg);
+ g_autofree char *url = soup_uri_to_string (uri, FALSE);
+ g_warning ("Result of downloading screenshot from "
+ "'%s': %s", url,
+ soup_status_get_phrase (msg->status_code));
+
+ /* if we are already showing a screenshot, do not show and
+ * error */
+ if (ssimg->status == GS_SCREENSHOT_IMAGE_STATUS_IMAGE)
+ return;
+
/* TRANSLATORS: this is when we try to download a screenshot and
* we get back 404 */
gs_screenshot_image_set_error (ssimg, _("Screenshot not found"));
@@ -232,6 +317,8 @@ gs_screenshot_image_complete_cb (SoupSession *session,
/* load the image */
pixbuf = gdk_pixbuf_new_from_stream (stream, NULL, NULL);
if (pixbuf == NULL) {
+ if (ssimg->status == GS_SCREENSHOT_IMAGE_STATUS_IMAGE)
+ return;
/* TRANSLATORS: possibly image file corrupt or not an image */
gs_screenshot_image_set_error (ssimg, _("Failed to load image"));
return;
@@ -250,18 +337,30 @@ gs_screenshot_image_complete_cb (SoupSession *session,
return;
}
} else {
+ AsImageKind kind;
+
/* save to file, using the same code as the AppStream builder
* so the preview looks the same */
im = as_image_new ();
as_image_set_pixbuf (im, pixbuf);
+
ret = as_image_save_filename (im, ssimg->filename,
ssimg->width * ssimg->scale,
ssimg->height * ssimg->scale,
- AS_IMAGE_SAVE_FLAG_PAD_16_9, &error);
+ AS_IMAGE_SAVE_FLAG_PAD_16_9,
+ &error);
if (!ret) {
gs_screenshot_image_set_error (ssimg, error->message);
return;
}
+
+ /* if the kind is unknown then we save also the counterpart size
+ * image in order to save an extra download when the user
+ * requests that size later */
+ kind = as_image_get_kind (im);
+ if (kind == AS_IMAGE_KIND_UNKNOWN) {
+ gs_screenshot_image_save_counterpart (ssimg, im);
+ }
}
/* got image, so show */
@@ -324,6 +423,9 @@ gs_screenshot_image_load_async (GsScreenshotImage *ssimg,
g_autofree gchar *cachefn_thumb = NULL;
g_autofree gchar *sizedir = NULL;
g_autoptr(SoupURI) base_uri = NULL;
+ char *loaded_filename = NULL;
+ g_autofree char *writable_filename = NULL;
+ gboolean showing_image = FALSE;
g_return_if_fail (GS_IS_SCREENSHOT_IMAGE (ssimg));
@@ -359,6 +461,7 @@ gs_screenshot_image_load_async (GsScreenshotImage *ssimg,
as_screenshot_show_image (ssimg);
return;
}
+ g_clear_pointer (&ssimg->filename, g_free);
}
basename = gs_screenshot_get_cachefn_for_url (url);
@@ -368,26 +471,42 @@ gs_screenshot_image_load_async (GsScreenshotImage *ssimg,
sizedir = g_strdup_printf ("%ux%u", ssimg->width * ssimg->scale, ssimg->height *
ssimg->scale);
}
cache_kind = g_build_filename ("screenshots", sizedir, NULL);
- ssimg->filename = gs_utils_get_cache_filename (cache_kind,
- basename,
- GS_UTILS_CACHE_FLAG_NONE,
- NULL);
- if (ssimg->filename == NULL) {
+
+ /* request the writable cache filename before trying the system's one
+ * because the former should be the more recent */
+ writable_filename = gs_utils_get_cache_filename (cache_kind,
+ basename,
+ GS_UTILS_CACHE_FLAG_WRITEABLE,
+ NULL);
+
+ if (writable_filename == NULL) {
/* TRANSLATORS: this is when we try create the cache directory
* but we were out of space or permission was denied */
gs_screenshot_image_set_error (ssimg, _("Could not create cache"));
return;
}
- /* does local file already exist */
- if (g_file_test (ssimg->filename, G_FILE_TEST_EXISTS)) {
- as_screenshot_show_image (ssimg);
- return;
+ if (ssimg->filename != NULL) {
+ g_free (ssimg->filename);
+ }
+
+ /* if the user's cache file does not exist, try to use the system one */
+ if (!g_file_test (writable_filename, G_FILE_TEST_EXISTS)) {
+ ssimg->filename = gs_utils_get_cache_filename (cache_kind,
+ basename,
+ GS_UTILS_CACHE_FLAG_NONE,
+ NULL);
+ } else {
+ ssimg->filename = g_steal_pointer (&writable_filename);
}
- /* can we load a blurred smaller version of this straight away */
- if (ssimg->width > AS_IMAGE_THUMBNAIL_WIDTH &&
- ssimg->height > AS_IMAGE_THUMBNAIL_HEIGHT) {
+ /* show the screenshot if its cached version exists, otherwise try to
+ * load a blurred smaller version while we fetch the new image */
+ if (g_file_test (ssimg->filename, G_FILE_TEST_EXISTS)) {
+ as_screenshot_show_image (ssimg);
+ showing_image = TRUE;
+ } else if (ssimg->width > AS_IMAGE_THUMBNAIL_WIDTH &&
+ ssimg->height > AS_IMAGE_THUMBNAIL_HEIGHT) {
const gchar *url_thumb;
g_autofree gchar *basename_thumb = NULL;
g_autofree gchar *cache_kind_thumb = NULL;
@@ -407,18 +526,16 @@ gs_screenshot_image_load_async (GsScreenshotImage *ssimg,
gs_screenshot_image_show_blurred (ssimg, cachefn_thumb);
}
- /* re-request the cache filename, which might be different as it needs
- * to be writable this time */
- g_free (ssimg->filename);
- ssimg->filename = gs_utils_get_cache_filename (cache_kind,
- basename,
- GS_UTILS_CACHE_FLAG_WRITEABLE,
- NULL);
-
- /* download file */
- g_debug ("downloading %s to %s", url, ssimg->filename);
+ /* download file (if there is a new version available) */
base_uri = soup_uri_new (url);
if (base_uri == NULL || !SOUP_URI_VALID_FOR_HTTP (base_uri)) {
+ if (showing_image) {
+ g_warning ("Could not try to download a new screenshot "
+ "because the URL is invalid '%s'. Showing "
+ "cached image '%s'.", url, ssimg->filename);
+ return;
+ }
+
/* TRANSLATORS: this is when we try to download a screenshot
* that was not a valid URL */
gs_screenshot_image_set_error (ssimg, _("Screenshot not valid"));
@@ -435,12 +552,33 @@ gs_screenshot_image_load_async (GsScreenshotImage *ssimg,
ssimg->message = soup_message_new_from_uri (SOUP_METHOD_GET, base_uri);
if (ssimg->message == NULL) {
+ if (showing_image) {
+ g_warning ("Could not setup a new soup_message for the "
+ "URL '%s'. Showing cached image '%s'.", url,
+ ssimg->filename);
+ return;
+ }
+
+ /* silently return as we have already a screenshot set, even if
+ * eventually outdated */
+ if (showing_image)
+ return;
+
/* TRANSLATORS: this is when networking is not available */
gs_screenshot_image_set_error (ssimg, _("Screenshot not available"));
return;
}
+ if (showing_image)
+ gs_utils_set_modified_request (ssimg->message, ssimg->filename);
+
+ if (writable_filename != NULL) {
+ g_free (ssimg->filename);
+ ssimg->filename = g_steal_pointer (&writable_filename);
+ }
+
/* send async */
+ g_debug ("Downloading '%s' if its newer than our cached file.", url);
soup_session_queue_message (ssimg->session,
g_object_ref (ssimg->message) /* transfer full */,
gs_screenshot_image_complete_cb,
@@ -481,6 +619,7 @@ gs_screenshot_image_init (GsScreenshotImage *ssimg)
atk_object_set_role (accessible, ATK_ROLE_IMAGE);
atk_object_set_name (accessible, _("Screenshot"));
}
+ ssimg->status = GS_SCREENSHOT_IMAGE_STATUS_UNKNOWN;
}
static gboolean
diff --git a/src/gs-utils.c b/src/gs-utils.c
index ac9bbd2..c900a77 100644
--- a/src/gs-utils.c
+++ b/src/gs-utils.c
@@ -796,4 +796,31 @@ gs_utils_error_convert_appstream (GError **perror)
return TRUE;
}
+gboolean
+gs_utils_set_modified_request (SoupMessage *msg,
+ const char *file_path)
+{
+ GTimeVal time_val;
+ g_autoptr(GDateTime) date_time = NULL;
+ g_autoptr(GFile) file = NULL;
+ g_autoptr(GFileInfo) info = NULL;
+ g_autofree char *mod_date = NULL;
+
+ file = g_file_new_for_path (file_path);
+ info = g_file_query_info (file,
+ G_FILE_ATTRIBUTE_TIME_MODIFIED,
+ G_FILE_QUERY_INFO_NONE,
+ NULL,
+ NULL);
+ if (info == NULL)
+ return FALSE;
+ g_file_info_get_modification_time (info, &time_val);
+ date_time = g_date_time_new_from_timeval_local (&time_val);
+ mod_date = g_date_time_format (date_time, "%a, %d %b %Y %H:%M:%S %Z");
+
+ soup_message_headers_append (msg->request_headers, "If-Modified-Since",
+ mod_date);
+ return TRUE;
+}
+
/* vim: set noexpandtab: */
diff --git a/src/gs-utils.h b/src/gs-utils.h
index 3a55cd3..ffa2189 100644
--- a/src/gs-utils.h
+++ b/src/gs-utils.h
@@ -24,6 +24,7 @@
#include <gio/gdesktopappinfo.h>
#include <gtk/gtk.h>
+#include <libsoup/soup.h>
#include "gs-app.h"
@@ -78,6 +79,8 @@ gboolean gs_utils_error_convert_gdbus (GError **perror);
gboolean gs_utils_error_convert_gdk_pixbuf(GError **perror);
gboolean gs_utils_error_convert_json_glib (GError **perror);
gboolean gs_utils_error_convert_appstream (GError **perror);
+gboolean gs_utils_set_modified_request (SoupMessage *message,
+ const char *file_path);
G_END_DECLS
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
