Merge pull request #8130 from nextcloud/fix-own-avatar-not-updated-when-display-name-is-changed

[stable12] Fix own avatar not updated when display name is changed

2 files changed, 135 insertions, 3 deletions

diff --git a/lib/private/AvatarManager.php b/lib/private/AvatarManager.php
index b8c6c2a1eb6..da3ac62097d 100644
--- a/lib/private/AvatarManager.php
+++ b/lib/private/AvatarManager.php
@@ -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);
+		}
+	}
 }
diff --git a/tests/lib/AvatarManagerTest.php b/tests/lib/AvatarManagerTest.php
index 83a5cfd9b30..df1367a28d3 100644
--- a/tests/lib/AvatarManagerTest.php
+++ b/tests/lib/AvatarManagerTest.php
@@ -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');
+	}
 }