Fix own avatar not updated when display name is changed
The avatar endpoint returns the avatar image or, if the user has no avatar, the display name. In that later case the avatar is generated on the browser based on the display name. The avatar endpoint response is cached, so when the display name changes and the avatar is got again the browser could use the cached value, in which case it would use the same display name as before and the avatar would not change. When the avatar is an image the cache is invalidated with the use of the "version" parameter, which is increased when the image changes. When the avatar cache was first introduced only the image avatars were cached, but it was later changed to cache all avatar responses to limit the requests made to the server. Thus, now the cache of the display name is invalidated too by increasing the version of the avatar if the display name changes and there is no explicit avatar set. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
This commit is contained in:
parent
2fa5466b6c
commit
16b0b3f418
|
@ -75,6 +75,12 @@ class AvatarManager implements IAvatarManager {
|
|||
$this->l = $l;
|
||||
$this->logger = $logger;
|
||||
$this->config = $config;
|
||||
|
||||
$this->userManager->listen('\OC\User', 'changeUser', function ($user, $feature, $value, $oldValue) {
|
||||
if ($feature === 'displayName') {
|
||||
$this->updateAvatarVersionOnDisplayNameChange($user);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -102,4 +108,22 @@ class AvatarManager implements IAvatarManager {
|
|||
|
||||
return new Avatar($folder, $this->l, $user, $this->logger, $this->config);
|
||||
}
|
||||
|
||||
/**
|
||||
* Increases the avatar version if needed.
|
||||
*
|
||||
* When a user has no avatar set the avatar is generated in the client based
|
||||
* on the display name, so in that case a display name change acts like an
|
||||
* avatar change and its version has to be increased.
|
||||
*
|
||||
* @param \OC\User\User $user the user whose display name has changed
|
||||
*/
|
||||
private function updateAvatarVersionOnDisplayNameChange(\OC\User\User $user) {
|
||||
$avatar = $this->getAvatar($user->getUID());
|
||||
|
||||
if (!$avatar->exists()) {
|
||||
$this->config->setUserValue($user->getUID(), 'avatar', 'version',
|
||||
(int)$this->config->getUserValue($user->getUID(), 'avatar', 'version', 0) + 1);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -27,19 +27,21 @@ namespace Test;
|
|||
use OC\Avatar;
|
||||
use OC\AvatarManager;
|
||||
use OC\Files\AppData\AppData;
|
||||
use OC\User\Manager as UserManager;
|
||||
use OC\User\User;
|
||||
use OCP\Files\IAppData;
|
||||
use OCP\Files\SimpleFS\ISimpleFolder;
|
||||
use OCP\IAvatar;
|
||||
use OCP\IConfig;
|
||||
use OCP\IL10N;
|
||||
use OCP\ILogger;
|
||||
use OCP\IUser;
|
||||
use OCP\IUserManager;
|
||||
|
||||
/**
|
||||
* Class AvatarManagerTest
|
||||
*/
|
||||
class AvatarManagerTest extends \Test\TestCase {
|
||||
/** @var IUserManager|\PHPUnit_Framework_MockObject_MockObject */
|
||||
/** @var UserManager|\PHPUnit_Framework_MockObject_MockObject */
|
||||
private $userManager;
|
||||
/** @var IAppData|\PHPUnit_Framework_MockObject_MockObject */
|
||||
private $appData;
|
||||
|
@ -49,18 +51,33 @@ class AvatarManagerTest extends \Test\TestCase {
|
|||
private $logger;
|
||||
/** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */
|
||||
private $config;
|
||||
/** @var callable **/
|
||||
private $changeUserCallback;
|
||||
/** @var AvatarManager | \PHPUnit_Framework_MockObject_MockObject */
|
||||
private $avatarManager;
|
||||
|
||||
public function setUp() {
|
||||
parent::setUp();
|
||||
|
||||
$this->userManager = $this->createMock(IUserManager::class);
|
||||
$this->userManager = $this->createMock(UserManager::class);
|
||||
$this->appData = $this->createMock(IAppData::class);
|
||||
$this->l10n = $this->createMock(IL10N::class);
|
||||
$this->logger = $this->createMock(ILogger::class);
|
||||
$this->config = $this->createMock(IConfig::class);
|
||||
|
||||
$this->userManager
|
||||
->expects($this->any())
|
||||
->method('listen')
|
||||
->with(
|
||||
'\OC\User',
|
||||
'changeUser',
|
||||
$this->callback(function($changeUserCallback) {
|
||||
$this->changeUserCallback = $changeUserCallback;
|
||||
|
||||
return true;
|
||||
})
|
||||
);
|
||||
|
||||
$this->avatarManager = new AvatarManager(
|
||||
$this->userManager,
|
||||
$this->appData,
|
||||
|
@ -127,4 +144,95 @@ class AvatarManagerTest extends \Test\TestCase {
|
|||
$expected = new Avatar($folder, $this->l10n, $user, $this->logger, $this->config);
|
||||
$this->assertEquals($expected, $this->avatarManager->getAvatar('vaLid-USER'));
|
||||
}
|
||||
|
||||
public function testUpdateAvatarVersionOnDisplayNameChange() {
|
||||
$this->avatarManager =
|
||||
$this->getMockBuilder(AvatarManager::class)
|
||||
->setMethods(['getAvatar'])
|
||||
->setConstructorArgs([
|
||||
$this->userManager,
|
||||
$this->appData,
|
||||
$this->l10n,
|
||||
$this->logger,
|
||||
$this->config
|
||||
])->getMock();
|
||||
|
||||
$user = $this->createMock(User::class);
|
||||
|
||||
$user->expects($this->any())
|
||||
->method('getUID')
|
||||
->will($this->returnValue(42));
|
||||
|
||||
$avatar = $this->createMock(IAvatar::class);
|
||||
|
||||
$this->avatarManager
|
||||
->expects($this->once())
|
||||
->method('getAvatar')
|
||||
->with(42)
|
||||
->will($this->returnValue($avatar));
|
||||
|
||||
$avatar->expects($this->once())
|
||||
->method('exists')
|
||||
->will($this->returnValue(false));
|
||||
|
||||
$this->config
|
||||
->expects($this->once())
|
||||
->method('getUserValue')
|
||||
->with(42, 'avatar', 'version', 0)
|
||||
->will($this->returnValue(15));
|
||||
|
||||
$this->config
|
||||
->expects($this->once())
|
||||
->method('setUserValue')
|
||||
->with(42, 'avatar', 'version', 16);
|
||||
|
||||
call_user_func($this->changeUserCallback, $user, 'displayName', 'newValue', 'oldValue');
|
||||
}
|
||||
|
||||
public function testUpdateAvatarVersionOnDisplayNameChangeUserWithAvatar() {
|
||||
$this->avatarManager =
|
||||
$this->getMockBuilder(AvatarManager::class)
|
||||
->setMethods(['getAvatar'])
|
||||
->setConstructorArgs([
|
||||
$this->userManager,
|
||||
$this->appData,
|
||||
$this->l10n,
|
||||
$this->logger,
|
||||
$this->config
|
||||
])->getMock();
|
||||
|
||||
$user = $this->createMock(User::class);
|
||||
|
||||
$user->expects($this->any())
|
||||
->method('getUID')
|
||||
->will($this->returnValue(42));
|
||||
|
||||
$avatar = $this->createMock(IAvatar::class);
|
||||
|
||||
$this->avatarManager
|
||||
->expects($this->once())
|
||||
->method('getAvatar')
|
||||
->with(42)
|
||||
->will($this->returnValue($avatar));
|
||||
|
||||
$avatar->expects($this->once())
|
||||
->method('exists')
|
||||
->will($this->returnValue(true));
|
||||
|
||||
$this->config
|
||||
->expects($this->never())
|
||||
->method('setUserValue');
|
||||
|
||||
call_user_func($this->changeUserCallback, $user, 'displayName', 'newValue', 'oldValue');
|
||||
}
|
||||
|
||||
public function testUpdateAvatarVersionOnDisplayNameChangeDifferentChangedAttribute() {
|
||||
$user = $this->createMock(User::class);
|
||||
|
||||
$this->config
|
||||
->expects($this->never())
|
||||
->method('setUserValue');
|
||||
|
||||
call_user_func($this->changeUserCallback, $user, 'otherAttribute', 'newValue', 'oldValue');
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue