Backport of #12054 to stable13

only write when the displayname differs, but then announce it

refs #5212 and fixes #9112

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>

do not run into UniqueConstraintViolationException

… when an unmapped user logs in for the first time when background job
mode is ajax and no memcache was configured.

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>

2 files changed, 50 insertions, 6 deletions

diff --git a/apps/user_ldap/lib/User/User.php b/apps/user_ldap/lib/User/User.php
index 32cf4812461..7c5edcad6b0 100644
--- a/apps/user_ldap/lib/User/User.php
+++ b/apps/user_ldap/lib/User/User.php
@@ -414,14 +414,23 @@ class User {
 	 *
 	 * @param string $displayName
 	 * @param string $displayName2
-	 * @returns string the effective display name
+	 * @return string the effective display name
 	 */
 	public function composeAndStoreDisplayName($displayName, $displayName2 = '') {
 		$displayName2 = strval($displayName2);
 		if($displayName2 !== '') {
 			$displayName .= ' (' . $displayName2 . ')';
 		}
-		$this->store('displayName', $displayName);
+		$oldName = $this->config->getUserValue($this->uid, 'user_ldap', 'displayName', null);
+		if ($oldName !== $displayName)  {
+			$this->store('displayName', $displayName);
+			$user = $this->userManager->get($this->getUsername());
+			if (!empty($oldName) && $user instanceof \OC\User\User) {
+				// if it was empty, it would be a new record, not a change emitting the trigger could
+				// potentially cause a UniqueConstraintViolationException, depending on some factors.
+				$user->triggerChange('displayName', $displayName);
+			}
+		}
 		return $displayName;
 	}
 
diff --git a/apps/user_ldap/tests/User/UserTest.php b/apps/user_ldap/tests/User/UserTest.php
index 29d2ba3829f..7970c1e80c8 100644
--- a/apps/user_ldap/tests/User/UserTest.php
+++ b/apps/user_ldap/tests/User/UserTest.php
@@ -1232,21 +1232,41 @@ class UserTest extends \Test\TestCase {
 
 	public function displayNameProvider() {
 		return [
-			['Roland Deschain', '', 'Roland Deschain'],
-			['Roland Deschain', null, 'Roland Deschain'],
-			['Roland Deschain', 'gunslinger@darktower.com', 'Roland Deschain (gunslinger@darktower.com)'],
+			['Roland Deschain', '', 'Roland Deschain', false],
+			['Roland Deschain', '', 'Roland Deschain', true],
+			['Roland Deschain', null, 'Roland Deschain', false],
+			['Roland Deschain', 'gunslinger@darktower.com', 'Roland Deschain (gunslinger@darktower.com)', false],
+			['Roland Deschain', 'gunslinger@darktower.com', 'Roland Deschain (gunslinger@darktower.com)', true],
 		];
 	}
 
 	/**
 	 * @dataProvider displayNameProvider
 	 */
-	public function testComposeAndStoreDisplayName($part1, $part2, $expected) {
+	public function testComposeAndStoreDisplayName($part1, $part2, $expected, $expectTriggerChange) {
 		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
 		$config->expects($this->once())
 			->method('setUserValue');
+		$oldName = $expectTriggerChange ? 'xxGunslingerxx' : null;
+		$this->config->expects($this->once())
+			->method('getUserValue')
+			->with($this->user->getUsername(), 'user_ldap', 'displayName', null)
+			->willReturn($oldName);
+
+		$ncUserObj = $this->createMock(\OC\User\User::class);
+		if ($expectTriggerChange) {
+			$ncUserObj->expects($this->once())
+				->method('triggerChange')
+				->with('displayName', $expected);
+		} else {
+			$ncUserObj->expects($this->never())
+				->method('triggerChange');
+		}
+		$this->userManager->expects($this->once())
+			->method('get')
+			->willReturn($ncUserObj);
 
 		$user = new User(
 			'user', 'cn=user', $this->access, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr);
@@ -1255,6 +1275,21 @@ class UserTest extends \Test\TestCase {
 		$this->assertSame($expected, $displayName);
 	}
 
+	public function testComposeAndStoreDisplayNameNoOverwrite() {
+		$displayName = 'Randall Flagg';
+		$this->config->expects($this->never())
+			->method('setUserValue');
+		$this->config->expects($this->once())
+			->method('getUserValue')
+			->willReturn($displayName);
+
+		$this->userManager->expects($this->never())
+			->method('get'); // Implicit: no triggerChange can be called
+
+		$composedDisplayName = $this->user->composeAndStoreDisplayName($displayName);
+		$this->assertSame($composedDisplayName, $displayName);
+	}
+
 	public function testHandlePasswordExpiryWarningDefaultPolicy() {
 		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();