Backport of #12054 to stable14

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>

diff --git a/apps/user_ldap/lib/User/User.php b/apps/user_ldap/lib/User/User.php
index 02764a72ecab61efaf5c542060135aeb2910a828..f4be19a7ad532f81495665c5d7e68a76c9cfb038 100644 (file)
--- 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 = (string)$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 837c72a3a31364b3c3f141fd43be9246ccf98812..6ff9defe47bcf54a248bb49253de0baa6a4de9a8 100644 (file)
--- a/apps/user_ldap/tests/User/UserTest.php
+++ b/apps/user_ldap/tests/User/UserTest.php
@@ -998,23 +998,58 @@ 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) {
                $this->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);
 
                $displayName = $this->user->composeAndStoreDisplayName($part1, $part2);
                $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() {
                $this->connection->expects($this->any())
                        ->method('__get')