Skip to content

Commit

Permalink
Backport of #12054 to stable14
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
blizzz committed Oct 30, 2018
1 parent 500b5d8 commit bdb8e37
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 6 deletions.
13 changes: 11 additions & 2 deletions apps/user_ldap/lib/User/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -414,14 +414,23 @@ private function store($key, $value) {
*
* @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;
}

Expand Down
43 changes: 39 additions & 4 deletions apps/user_ldap/tests/User/UserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -998,23 +998,58 @@ public function testGetHomePathConfiguredNotAvailableNotAllowed() {

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')
Expand Down

0 comments on commit bdb8e37

Please sign in to comment.