Skip to content

[stable13] LDAP: announce display name changes so that addressbook picks it up #12142

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rullzer merged 2 commits into from
Oct 30, 2018
Merged

[stable13] LDAP: announce display name changes so that addressbook picks it up #12142

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
908e6be
Backport of #12054 to stable13
blizzz Oct 25, 2018
82bb002
adjust tests to 13 codebase
blizzz Oct 30, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
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 <[email protected]>

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 <[email protected]>
  • Loading branch information
@blizzz
blizzz committed Oct 30, 2018
commit 908e6be07effdd3f47b70c8f5b12f1ea88b1d871
13 changes: 11 additions & 2 deletions apps/user_ldap/lib/User/User.php
View file
Open in desktop
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 = 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;
}

Expand Down
43 changes: 39 additions & 4 deletions apps/user_ldap/tests/User/UserTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1232,21 +1232,41 @@ public function testGetHomePathConfiguredNotAvailableNotAllowed() {

public function displayNameProvider() {
return [
['Roland Deschain', '', 'Roland Deschain'],
['Roland Deschain', null, 'Roland Deschain'],
['Roland Deschain', '[email protected]', 'Roland Deschain ([email protected])'],
['Roland Deschain', '', 'Roland Deschain', false],
['Roland Deschain', '', 'Roland Deschain', true],
['Roland Deschain', null, 'Roland Deschain', false],
['Roland Deschain', '[email protected]', 'Roland Deschain ([email protected])', false],
['Roland Deschain', '[email protected]', 'Roland Deschain ([email protected])', 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);
Expand All @@ -1255,6 +1275,21 @@ public function testComposeAndStoreDisplayName($part1, $part2, $expected) {
$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();
Expand Down