From 36618b111fde55ba5c8ddedc294e58927257d107 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Thu, 11 Apr 2019 09:49:21 +0200 Subject: [PATCH 1/2] Pass old value to user triggerChange hook Signed-off-by: Morris Jobke --- apps/user_ldap/lib/User/User.php | 2 +- lib/private/User/User.php | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/apps/user_ldap/lib/User/User.php b/apps/user_ldap/lib/User/User.php index d68d8b35d1..5c89950e08 100644 --- a/apps/user_ldap/lib/User/User.php +++ b/apps/user_ldap/lib/User/User.php @@ -436,7 +436,7 @@ class User { 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); + $user->triggerChange('displayName', $displayName, $oldName); } } return $displayName; diff --git a/lib/private/User/User.php b/lib/private/User/User.php index 17fa022b1b..a47ef8dc54 100644 --- a/lib/private/User/User.php +++ b/lib/private/User/User.php @@ -145,9 +145,8 @@ class User implements IUser { $this->triggerChange('displayName', $displayName); } return $result !== false; - } else { - return false; } + return false; } /** @@ -365,7 +364,8 @@ class User implements IUser { $oldStatus = $this->isEnabled(); $this->enabled = $enabled; if ($oldStatus !== $this->enabled) { - $this->triggerChange('enabled', $enabled); + // TODO: First change the value, then trigger the event as done for all other properties. + $this->triggerChange('enabled', $enabled, $oldStatus); $this->config->setUserValue($this->uid, 'core', 'enabled', $enabled ? 'true' : 'false'); } } @@ -409,7 +409,7 @@ class User implements IUser { } $this->config->setUserValue($this->uid, 'files', 'quota', $quota); if($quota !== $oldQuota) { - $this->triggerChange('quota', $quota); + $this->triggerChange('quota', $quota, $oldQuota); } } From f420647add0f5ffc917301335d82d951a1df502e Mon Sep 17 00:00:00 2001 From: Leon Klingele Date: Wed, 6 Mar 2019 13:07:41 +0100 Subject: [PATCH 2/2] lib/private/User: do not change user properties if value has not changed Signed-off-by: Morris Jobke --- lib/private/User/User.php | 12 ++++++------ tests/lib/User/UserTest.php | 20 ++++---------------- 2 files changed, 10 insertions(+), 22 deletions(-) diff --git a/lib/private/User/User.php b/lib/private/User/User.php index a47ef8dc54..48c913db2a 100644 --- a/lib/private/User/User.php +++ b/lib/private/User/User.php @@ -158,12 +158,12 @@ class User implements IUser { */ public function setEMailAddress($mailAddress) { $oldMailAddress = $this->getEMailAddress(); - if($mailAddress === '') { - $this->config->deleteUserValue($this->uid, 'settings', 'email'); - } else { - $this->config->setUserValue($this->uid, 'settings', 'email', $mailAddress); - } if($oldMailAddress !== $mailAddress) { + if($mailAddress === '') { + $this->config->deleteUserValue($this->uid, 'settings', 'email'); + } else { + $this->config->setUserValue($this->uid, 'settings', 'email', $mailAddress); + } $this->triggerChange('eMailAddress', $mailAddress, $oldMailAddress); } } @@ -407,8 +407,8 @@ class User implements IUser { $quota = OC_Helper::computerFileSize($quota); $quota = OC_Helper::humanFileSize($quota); } - $this->config->setUserValue($this->uid, 'files', 'quota', $quota); if($quota !== $oldQuota) { + $this->config->setUserValue($this->uid, 'files', 'quota', $quota); $this->triggerChange('quota', $quota, $oldQuota); } } diff --git a/tests/lib/User/UserTest.php b/tests/lib/User/UserTest.php index 16fde814b8..a800e79037 100644 --- a/tests/lib/User/UserTest.php +++ b/tests/lib/User/UserTest.php @@ -676,14 +676,8 @@ class UserTest extends TestCase { $config->expects($this->any()) ->method('getUserValue') ->willReturn('foo@bar.com'); - $config->expects($this->once()) - ->method('setUserValue') - ->with( - 'foo', - 'settings', - 'email', - 'foo@bar.com' - ); + $config->expects($this->never()) + ->method('setUserValue'); $user = new User('foo', $backend, $this->dispatcher, $emitter, $config); $user->setEMailAddress('foo@bar.com'); @@ -741,14 +735,8 @@ class UserTest extends TestCase { $config->expects($this->any()) ->method('getUserValue') ->willReturn('23 TB'); - $config->expects($this->once()) - ->method('setUserValue') - ->with( - 'foo', - 'files', - 'quota', - '23 TB' - ); + $config->expects($this->never()) + ->method('setUserValue'); $user = new User('foo', $backend, $this->dispatcher, $emitter, $config); $user->setQuota('23 TB');