From 14136295b7568c6e34504c101eba0ee10f5c74fd Mon Sep 17 00:00:00 2001
From: Roeland Jago Douma <roeland@famdouma.nl>
Date: Mon, 29 Aug 2016 14:55:23 +0200
Subject: [PATCH] Cache avatars properly
* Set proper caching headers for avatars (15 minutes)
* For our own avatar use some extra logic to invalidate when we update
---
core/Controller/AvatarController.php | 34 +++++++++++++++++++----
core/js/config.php | 14 ++++++++--
core/js/jquery.avatar.js | 23 ++++++++++++---
core/templates/layout.user.php | 4 +--
lib/private/Avatar.php | 14 +++++++++-
lib/private/AvatarManager.php | 11 ++++++--
lib/private/Server.php | 3 +-
lib/private/TemplateLayout.php | 1 +
lib/public/AppFramework/Http/Response.php | 3 +-
settings/js/personal.js | 13 +++++++--
10 files changed, 98 insertions(+), 22 deletions(-)
diff --git a/core/Controller/AvatarController.php b/core/Controller/AvatarController.php
index 5b64320948a..484b6f524bf 100644
--- a/core/Controller/AvatarController.php
+++ b/core/Controller/AvatarController.php
@@ -27,6 +27,7 @@
*/
namespace OC\Core\Controller;
+use OC\AppFramework\Utility\TimeFactory;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\DataDisplayResponse;
@@ -73,6 +74,9 @@ class AvatarController extends Controller {
/** @var string */
protected $userId;
+ /** @var TimeFactory */
+ protected $timeFactory;
+
/**
* @param string $appName
* @param IRequest $request
@@ -83,6 +87,7 @@ class AvatarController extends Controller {
* @param IRootFolder $rootFolder
* @param ILogger $logger
* @param string $userId
+ * @param TimeFactory $timeFactory
*/
public function __construct($appName,
IRequest $request,
@@ -92,7 +97,8 @@ class AvatarController extends Controller {
IUserManager $userManager,
IRootFolder $rootFolder,
ILogger $logger,
- $userId) {
+ $userId,
+ TimeFactory $timeFactory) {
parent::__construct($appName, $request);
$this->avatarManager = $avatarManager;
@@ -102,6 +108,7 @@ class AvatarController extends Controller {
$this->rootFolder = $rootFolder;
$this->logger = $logger;
$this->userId = $userId;
+ $this->timeFactory = $timeFactory;
}
/**
@@ -126,6 +133,21 @@ class AvatarController extends Controller {
Http::STATUS_OK,
['Content-Type' => $avatar->getMimeType()]);
$resp->setETag($avatar->getEtag());
+
+ // Let cache this!
+ $resp->addHeader('Pragma', 'public');
+ // Cache for 15 minutes
+ $resp->cacheFor(900);
+ // Set last modified
+ $lastModified = new \DateTime();
+ $lastModified->setTimestamp($avatar->getMTime());
+ $resp->setLastModified($lastModified);
+
+ $expires = new \DateTime();
+ $expires->setTimestamp($this->timeFactory->getTime());
+ $expires->add(new \DateInterval('PT15M'));
+ $resp->addHeader('Expires', $expires->format(\DateTime::RFC2822));
+
} catch (NotFoundException $e) {
$user = $this->userManager->get($userId);
$resp = new JSONResponse([
@@ -133,18 +155,20 @@ class AvatarController extends Controller {
'displayname' => $user->getDisplayName(),
],
]);
+ // Don't cache this
+ $resp->cacheFor(0);
+ $resp->setLastModified(new \DateTime('now', new \DateTimeZone('GMT')));
} catch (\Exception $e) {
$resp = new JSONResponse([
'data' => [
'displayname' => '',
],
]);
+ // Don't cache this
+ $resp->cacheFor(0);
+ $resp->setLastModified(new \DateTime('now', new \DateTimeZone('GMT')));
}
- $resp->addHeader('Pragma', 'public');
- $resp->cacheFor(0);
- $resp->setLastModified(new \DateTime('now', new \DateTimeZone('GMT')));
-
return $resp;
}
diff --git a/core/js/config.php b/core/js/config.php
index e88d9c741a1..8e85dc91cb1 100644
--- a/core/js/config.php
+++ b/core/js/config.php
@@ -149,8 +149,8 @@ $array = array(
"firstDay" => json_encode($l->l('firstday', null)) ,
"oc_config" => json_encode(
array(
- 'session_lifetime' => min(\OCP\Config::getSystemValue('session_lifetime', OC::$server->getIniWrapper()->getNumeric('session.gc_maxlifetime')), OC::$server->getIniWrapper()->getNumeric('session.gc_maxlifetime')),
- 'session_keepalive' => \OCP\Config::getSystemValue('session_keepalive', true),
+ 'session_lifetime' => min($config->getSystemValue('session_lifetime', OC::$server->getIniWrapper()->getNumeric('session.gc_maxlifetime')), OC::$server->getIniWrapper()->getNumeric('session.gc_maxlifetime')),
+ 'session_keepalive' => $config->getSystemValue('session_keepalive', true),
'version' => implode('.', \OCP\Util::getVersion()),
'versionstring' => OC_Util::getVersionString(),
'enable_avatars' => \OC::$server->getConfig()->getSystemValue('enable_avatars', true) === true,
@@ -187,9 +187,17 @@ $array = array(
'longFooter' => $defaults->getLongFooter(),
'folder' => OC_Util::getTheme(),
)
- )
+ ),
);
+if (OC_User::getUser() !== null && OC_User::getUser() !== false) {
+ $array['oc_userconfig'] = json_encode([
+ 'avatar' => [
+ 'version' => (int)$config->getUserValue(OC_User::getUser(), 'avatar', 'version', 0),
+ ]
+ ]);
+}
+
// Allow hooks to modify the output values
OC_Hook::emit('\OCP\Config', 'js', array('array' => &$array));
diff --git a/core/js/jquery.avatar.js b/core/js/jquery.avatar.js
index 6ae9cf78a13..754400acd7b 100644
--- a/core/js/jquery.avatar.js
+++ b/core/js/jquery.avatar.js
@@ -74,10 +74,25 @@
user = String(user).replace(/\//g,'');
var $div = this;
+ var url;
- var url = OC.generateUrl(
- '/avatar/{user}/{size}',
- {user: user, size: Math.ceil(size * window.devicePixelRatio)});
+ // If this is our own avatar we have to use the version attribute
+ if (user === OC.getCurrentUser().uid) {
+ url = OC.generateUrl(
+ '/avatar/{user}/{size}?v={version}',
+ {
+ user: user,
+ size: Math.ceil(size * window.devicePixelRatio),
+ version: oc_userconfig.avatar.version
+ });
+ } else {
+ url = OC.generateUrl(
+ '/avatar/{user}/{size}',
+ {
+ user: user,
+ size: Math.ceil(size * window.devicePixelRatio)
+ });
+ }
// If the displayname is not defined we use the old code path
if (typeof(displayname) === 'undefined') {
@@ -122,7 +137,7 @@
$div.show();
$div.text('');
$div.append(img);
- }
+ };
img.width = size;
img.height = size;
diff --git a/core/templates/layout.user.php b/core/templates/layout.user.php
index 17f895bc17d..b5466b6fc05 100644
--- a/core/templates/layout.user.php
+++ b/core/templates/layout.user.php
@@ -66,8 +66,8 @@
<div class="avatardiv<?php if ($_['userAvatarSet']) { print_unescaped(' avatardiv-shown'); } else { print_unescaped('" style="display: none'); } ?>">
<?php if ($_['userAvatarSet']): ?>
<img alt="" width="32" height="32"
- src="<?php p(\OC::$server->getURLGenerator()->linkToRoute('core.avatar.getAvatar', ['userId' => $_['user_uid'], 'size' => 32]));?>"
- srcset="<?php p(\OC::$server->getURLGenerator()->linkToRoute('core.avatar.getAvatar', ['userId' => $_['user_uid'], 'size' => 64]));?> 2x, <?php p(\OC::$server->getURLGenerator()->linkToRoute('core.avatar.getAvatar', ['userId' => $_['user_uid'], 'size' => 128]));?> 4x"
+ src="<?php p(\OC::$server->getURLGenerator()->linkToRoute('core.avatar.getAvatar', ['userId' => $_['user_uid'], 'size' => 32, 'v' => $_['userAvatarVersion']]));?>"
+ srcset="<?php p(\OC::$server->getURLGenerator()->linkToRoute('core.avatar.getAvatar', ['userId' => $_['user_uid'], 'size' => 64, 'v' => $_['userAvatarVersion']]));?> 2x, <?php p(\OC::$server->getURLGenerator()->linkToRoute('core.avatar.getAvatar', ['userId' => $_['user_uid'], 'size' => 128, 'v' => $_['userAvatarVersion']]));?> 4x"
>
<?php endif; ?>
</div>
diff --git a/lib/private/Avatar.php b/lib/private/Avatar.php
index 80691774b64..9e8bd0136c2 100644
--- a/lib/private/Avatar.php
+++ b/lib/private/Avatar.php
@@ -34,6 +34,7 @@ use OCP\Files\File;
use OCP\Files\NotFoundException;
use OCP\Files\NotPermittedException;
use OCP\IAvatar;
+use OCP\IConfig;
use OCP\IImage;
use OCP\IL10N;
use OC_Image;
@@ -52,6 +53,8 @@ class Avatar implements IAvatar {
private $user;
/** @var ILogger */
private $logger;
+ /** @var IConfig */
+ private $config;
/**
* constructor
@@ -60,12 +63,18 @@ class Avatar implements IAvatar {
* @param IL10N $l
* @param User $user
* @param ILogger $logger
+ * @param IConfig $config
*/
- public function __construct (Folder $folder, IL10N $l, $user, ILogger $logger) {
+ public function __construct(Folder $folder,
+ IL10N $l,
+ $user,
+ ILogger $logger,
+ IConfig $config) {
$this->folder = $folder;
$this->l = $l;
$this->user = $user;
$this->logger = $logger;
+ $this->config = $config;
}
/**
@@ -137,6 +146,9 @@ class Avatar implements IAvatar {
$regex = '/^avatar\.([0-9]+\.)?(jpg|png)$/';
$avatars = $this->folder->getDirectoryListing();
+ $this->config->setUserValue($this->user->getUID(), 'avatar', 'version',
+ (int)$this->config->getUserValue($this->user->getUID(), 'avatar', 'version', 0) + 1);
+
foreach ($avatars as $avatar) {
if (preg_match($regex, $avatar->getName())) {
$avatar->delete();
diff --git a/lib/private/AvatarManager.php b/lib/private/AvatarManager.php
index e461a70608b..0eabc3a1754 100644
--- a/lib/private/AvatarManager.php
+++ b/lib/private/AvatarManager.php
@@ -30,6 +30,7 @@ namespace OC;
use OCP\Files\Folder;
use OCP\Files\NotFoundException;
use OCP\IAvatarManager;
+use OCP\IConfig;
use OCP\ILogger;
use OCP\IUserManager;
use OCP\Files\IRootFolder;
@@ -52,6 +53,9 @@ class AvatarManager implements IAvatarManager {
/** @var ILogger */
private $logger;
+ /** @var IConfig */
+ private $config;
+
/**
* AvatarManager constructor.
*
@@ -59,16 +63,19 @@ class AvatarManager implements IAvatarManager {
* @param IRootFolder $rootFolder
* @param IL10N $l
* @param ILogger $logger
+ * @param IConfig $config
*/
public function __construct(
IUserManager $userManager,
IRootFolder $rootFolder,
IL10N $l,
- ILogger $logger) {
+ ILogger $logger,
+ IConfig $config) {
$this->userManager = $userManager;
$this->rootFolder = $rootFolder;
$this->l = $l;
$this->logger = $logger;
+ $this->config = $config;
}
/**
@@ -94,6 +101,6 @@ class AvatarManager implements IAvatarManager {
/** @var Folder $folder */
$folder = $this->rootFolder->get($dir);
- return new Avatar($folder, $this->l, $user, $this->logger);
+ return new Avatar($folder, $this->l, $user, $this->logger, $this->config);
}
}
diff --git a/lib/private/Server.php b/lib/private/Server.php
index 6f6d403210d..494387ab6ca 100644
--- a/lib/private/Server.php
+++ b/lib/private/Server.php
@@ -361,7 +361,8 @@ class Server extends ServerContainer implements IServerContainer {
$c->getUserManager(),
$c->getRootFolder(),
$c->getL10N('lib'),
- $c->getLogger()
+ $c->getLogger(),
+ $c->getConfig()
);
});
$this->registerService('Logger', function (Server $c) {
diff --git a/lib/private/TemplateLayout.php b/lib/private/TemplateLayout.php
index 7e5c120717f..da845d80d04 100644
--- a/lib/private/TemplateLayout.php
+++ b/lib/private/TemplateLayout.php
@@ -112,6 +112,7 @@ class TemplateLayout extends \OC_Template {
$this->assign('userAvatarSet', false);
} else {
$this->assign('userAvatarSet', \OC::$server->getAvatarManager()->getAvatar(\OC_User::getUser())->exists());
+ $this->assign('userAvatarVersion', \OC::$server->getConfig()->getUserValue(\OC_User::getUser(), 'avatar', 'version', 0));
}
} else if ($renderAs == 'error') {
diff --git a/lib/public/AppFramework/Http/Response.php b/lib/public/AppFramework/Http/Response.php
index eed9aa7c1a2..0b162b453a0 100644
--- a/lib/public/AppFramework/Http/Response.php
+++ b/lib/public/AppFramework/Http/Response.php
@@ -92,8 +92,7 @@ class Response {
public function cacheFor($cacheSeconds) {
if($cacheSeconds > 0) {
- $this->addHeader('Cache-Control', 'max-age=' . $cacheSeconds .
- ', must-revalidate');
+ $this->addHeader('Cache-Control', 'max-age=' . $cacheSeconds . ', must-revalidate');
} else {
$this->addHeader('Cache-Control', 'no-cache, no-store, must-revalidate');
}
diff --git a/settings/js/personal.js b/settings/js/personal.js
index 16a8d184da6..e0c99ae774d 100644
--- a/settings/js/personal.js
+++ b/settings/js/personal.js
@@ -100,6 +100,9 @@ function updateAvatar (hidedefault) {
var $headerdiv = $('#header .avatardiv');
var $displaydiv = $('#displayavatar .avatardiv');
+ //Bump avatar avatarversion
+ oc_userconfig.avatar.version = -(Math.floor(Math.random() * 1000));
+
if (hidedefault) {
$headerdiv.hide();
$('#header .avatardiv').removeClass('avatardiv-shown');
@@ -112,7 +115,10 @@ function updateAvatar (hidedefault) {
$displaydiv.avatar(OC.currentUser, 145, true);
$.get(OC.generateUrl(
'/avatar/{user}/{size}',
- {user: OC.currentUser, size: 1}
+ {
+ user: OC.currentUser,
+ size: 1
+ }
), function (result) {
if (typeof(result) === 'string') {
// Show the delete button when the avatar is custom
@@ -358,7 +364,10 @@ $(document).ready(function () {
// does the user have a custom avatar? if he does show #removeavatar
$.get(OC.generateUrl(
'/avatar/{user}/{size}',
- {user: OC.currentUser, size: 1}
+ {
+ user: OC.currentUser,
+ size: 1
+ }
), function (result) {
if (typeof(result) === 'string') {
// Show the delete button when the avatar is custom
--
GitLab