Merge lp:~robru/friends/avatar-expiry-workaround into lp:friends

[Robert Bruce Park]

Stop expiring old avatars from the cache.

So Ken, in light of the fact that we are quite late in the freeze, and the "correct" avatar fix would require new API (and thus a FFe), I propose that we land this workaround, and then early in the S cycle we can worry about fixing this "properly".

This commit simply neuters our ExpireAvatar dbus method (without removing it, so nothing that uses that API needs to be altered), and then it adds a simple mtime check to the download logic, causing it to trigger a redownload even if the file is already in the cache. End result: cached avatars are never deleted, but stale ones are redownloaded monthly.

This way older posts in the stream will not be robbed of their Avatar images.

Next cycle we can look at a) adding new GetCachedAvatar dbus method, b) updating libfriends API, and c) updating gwibber to know that the avatar field no longer contains cached data but raw URLs. (I almost wonder if maybe Qml doesn't provide some kind of caching mechanism that might be a bit more streamlined than having to query DBus for cached downloads...)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:171
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~robru/friends/avatar-expiry-workaround/+merge/155653/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/friends-ci/12/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/friends-raring-amd64-ci/12

Click here to trigger a rebuild:
http://s-jenkins:8080/job/friends-ci/12/rebuild

review: Needs Fixing (continuous-integration)

[Ken VanDine (ken-vandine)]

Looks good, but needs a debian/changelog entry

review: Needs Fixing

[lp:~robru/friends/avatar-expiry-workaround]

172. By Robert Bruce Park on 2013-03-29

* Don't delete stale avatars, just redownload periodically (LP: #1153896)

[Robert Bruce Park (robru)]

Oops, ok. Please merge ;-)

review: Approve

[lp:~robru/friends/avatar-expiry-workaround]

173. By Robert Bruce Park on 2013-03-29

[ Robert Bruce Park ]
* Flickr photos are 404ing (LP: #1159979)
[ Ubuntu daily release ]
* Automatic snapshot from revision 170

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:172
http://jenkins.qa.ubuntu.com/job/friends-ci/14/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/friends-raring-amd64-ci/14

Click here to trigger a rebuild:
http://s-jenkins:8080/job/friends-ci/14/rebuild

review: Approve (continuous-integration)

[Ken VanDine (ken-vandine)]

review: Approve

=== modified file 'debian/changelog'
--- debian/changelog	2013-03-28 05:01:42 +0000
+++ debian/changelog	2013-03-29 16:07:23 +0000
@@ -1,3 +1,9 @@
+friends (0.1.3daily13.03.29-0ubuntu1) UNRELEASED; urgency=low
+
+  * Don't delete stale avatars, just redownload periodically (LP: #1153896)
+
+ -- Robert Bruce Park <[e-mail]>  Fri, 29 Mar 2013 08:58:45 -0700
+
 friends (0.1.3daily13.03.28-0ubuntu1) raring; urgency=low
 
   [ Robert Bruce Park ]
@@ -29,13 +35,13 @@
   [ Robert Bruce Park ]
   * Keep the Dispatcher alive for 30s beyond the return of the final
     method invocation.
-  * Stop deduplicating messages across protocols, simplifying model 
+  * Stop deduplicating messages across protocols, simplifying model
     schema (LP: #1156941)
   * Add schema columns for latitude, longitude, and location name.
   * Fix 'likes' column from gdouble to guint64.
   * Add geotagging support from foursquare, facebook, flickr.
   * Implement since= for Facebook, reducing bandwidth usage.
-  * Automatically prepend the required @mention to Twitter 
+  * Automatically prepend the required @mention to Twitter
     replies (LP: #1156829)
   * Automatically linkify URLs that get published to the model.
   * Fix the publishing of Facebook Stories (LP: #1155785)

=== modified file 'friends/service/dispatcher.py'
--- friends/service/dispatcher.py	2013-03-20 01:19:39 +0000
+++ friends/service/dispatcher.py	2013-03-29 16:07:23 +0000
@@ -358,4 +358,7 @@
     @exit_after_idle
     @dbus.service.method(DBUS_INTERFACE)
     def ExpireAvatars(self):
-        Avatar.expire_old_avatars()
+        pass
+        # Disabled because we don't currently have a way of re-fetching
+        # expired avatars if they're needed again later.
+        # Avatar.expire_old_avatars()

=== modified file 'friends/utils/avatar.py'
--- friends/utils/avatar.py	2013-02-27 16:52:19 +0000
+++ friends/utils/avatar.py	2013-03-29 16:07:23 +0000
@@ -33,7 +33,7 @@
 
 CACHE_DIR = os.path.realpath(os.path.join(
     GLib.get_user_cache_dir(), 'friends', 'avatars'))
-CACHE_AGE = timedelta(weeks=4)
+AGE_LIMIT = date.today() - timedelta(weeks=4)
 
 
 try:
@@ -60,13 +60,14 @@
         local_path = Avatar.get_path(url)
         try:
             size = os.stat(local_path).st_size
+            mtime = date.fromtimestamp(os.stat(local_path).st_mtime)
         except OSError as error:
             if error.errno != errno.ENOENT:
                 # Some other error occurred, so propagate it up.
                 raise
             # Treat a missing file as zero length.
             size = 0
-        if size == 0:
+        if size == 0 or mtime < AGE_LIMIT:
             log.debug('Getting: {}'.format(url))
             image_data = Downloader(url).get_bytes()
 
@@ -89,11 +90,10 @@
     def expire_old_avatars():
         """Evict old files from the cache."""
         log.debug('Checking if anything needs to expire.')
-        limit = date.today() - CACHE_AGE
         for filename in os.listdir(CACHE_DIR):
             path = os.path.join(CACHE_DIR, filename)
             mtime = date.fromtimestamp(os.stat(path).st_mtime)
-            if mtime < limit:
+            if mtime < AGE_LIMIT:
                 # The file's last modification time is earlier than the oldest
                 # time we'll allow in the cache.  However, due to race
                 # conditions, ignore it if the file has already been removed.