From aeb89947a2bddb1db10426538afaccdc141c059e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Mon, 18 Jan 2016 20:27:43 +0100 Subject: [PATCH] Introduce IUser::setEMailAddress and add hook mechanism --- apps/provisioning_api/lib/users.php | 2 +- apps/provisioning_api/tests/userstest.php | 6 +-- apps/user_ldap/lib/user/user.php | 3 ++ lib/private/server.php | 4 ++ lib/private/user/manager.php | 1 + lib/private/user/user.php | 19 +++++++++ lib/public/iuser.php | 9 +++++ settings/controller/userscontroller.php | 8 +--- tests/lib/contacts/localadressbook.php | 6 +++ tests/lib/user/user.php | 3 +- .../controller/userscontrollertest.php | 39 +++++++------------ 11 files changed, 63 insertions(+), 37 deletions(-) diff --git a/apps/provisioning_api/lib/users.php b/apps/provisioning_api/lib/users.php index a385f2c1180..efb10a50865 100644 --- a/apps/provisioning_api/lib/users.php +++ b/apps/provisioning_api/lib/users.php @@ -285,7 +285,7 @@ class Users { break; case 'email': if(filter_var($parameters['_put']['value'], FILTER_VALIDATE_EMAIL)) { - $this->config->setUserValue($targetUserId, 'settings', 'email', $parameters['_put']['value']); + $targetUser->setEMailAddress($parameters['_put']['value']); } else { return new OC_OCS_Result(null, 102); } diff --git a/apps/provisioning_api/tests/userstest.php b/apps/provisioning_api/tests/userstest.php index 25e723a13b4..3ce13181b8d 100644 --- a/apps/provisioning_api/tests/userstest.php +++ b/apps/provisioning_api/tests/userstest.php @@ -932,10 +932,10 @@ class UsersTest extends OriginalTest { ->method('get') ->with('UserToEdit') ->will($this->returnValue($targetUser)); - $this->config + $targetUser ->expects($this->once()) - ->method('setUserValue') - ->with('UserToEdit', 'settings', 'email', 'demo@owncloud.org'); + ->method('setEMailAddress') + ->with('demo@owncloud.org'); $expected = new \OC_OCS_Result(null, 100); $this->assertEquals($expected, $this->api->editUser(['userid' => 'UserToEdit', '_put' => ['key' => 'email', 'value' => 'demo@owncloud.org']])); diff --git a/apps/user_ldap/lib/user/user.php b/apps/user_ldap/lib/user/user.php index 23fd831b62f..a0efeedb886 100644 --- a/apps/user_ldap/lib/user/user.php +++ b/apps/user_ldap/lib/user/user.php @@ -400,6 +400,9 @@ class User { } } if(!is_null($email)) { + // + // TODO: user IUser::setEMailAddress() + // $this->config->setUserValue( $this->uid, 'settings', 'email', $email); } diff --git a/lib/private/server.php b/lib/private/server.php index 642267e8568..db36fbc1433 100644 --- a/lib/private/server.php +++ b/lib/private/server.php @@ -229,6 +229,10 @@ class Server extends ServerContainer implements IServerContainer { $userSession->listen('\OC\User', 'logout', function () { \OC_Hook::emit('OC_User', 'logout', array()); }); + $userSession->listen('\OC\User', 'changeUser', function ($user) { + /** @var $user \OC\User\User */ + \OC_Hook::emit('OC_User', 'changeUser', array('run' => true, 'user' => $user)); + }); return $userSession; }); $this->registerService('NavigationManager', function ($c) { diff --git a/lib/private/user/manager.php b/lib/private/user/manager.php index 2aafe98cef2..b547f981616 100644 --- a/lib/private/user/manager.php +++ b/lib/private/user/manager.php @@ -44,6 +44,7 @@ use OCP\IConfig; * - postDelete(\OC\User\User $user) * - preCreateUser(string $uid, string $password) * - postCreateUser(\OC\User\User $user, string $password) + * - change(\OC\User\User $user) * * @package OC\User */ diff --git a/lib/private/user/user.php b/lib/private/user/user.php index c2a6acc7664..d8a888806e0 100644 --- a/lib/private/user/user.php +++ b/lib/private/user/user.php @@ -145,6 +145,24 @@ class User implements IUser { } } + /** + * set the email address of the user + * + * @param string|null $mailAddress + * @return void + * @since 9.0.0 + */ + public function setEMailAddress($mailAddress) { + if($mailAddress === '') { + $this->config->deleteUserValue($this->uid, 'settings', 'email'); + } else { + $this->config->setUserValue($this->uid, 'settings', 'email', $mailAddress); + } + if ($this->emitter) { + $this->emitter->emit('\OC\User', 'changeUser', array($this)); + } + } + /** * returns the timestamp of the user's last login or 0 if the user did never * login @@ -365,4 +383,5 @@ class User implements IUser { return $url; } + } diff --git a/lib/public/iuser.php b/lib/public/iuser.php index 06921a1ee23..454d45eae76 100644 --- a/lib/public/iuser.php +++ b/lib/public/iuser.php @@ -169,4 +169,13 @@ interface IUser { * @since 9.0.0 */ public function getCloudId(); + + /** + * set the email address of the user + * + * @param string|null $mailAddress + * @return void + * @since 9.0.0 + */ + public function setEMailAddress($mailAddress); } diff --git a/settings/controller/userscontroller.php b/settings/controller/userscontroller.php index 695d72cfb5a..17629fe924f 100644 --- a/settings/controller/userscontroller.php +++ b/settings/controller/userscontroller.php @@ -373,7 +373,7 @@ class UsersController extends Controller { * Send new user mail only if a mail is set */ if($email !== '') { - $this->config->setUserValue($username, 'settings', 'email', $email); + $user->setEMailAddress($email); // data for the mail template $mailData = array( @@ -545,11 +545,7 @@ class UsersController extends Controller { } // delete user value if email address is empty - if($mailAddress === '') { - $this->config->deleteUserValue($id, 'settings', 'email'); - } else { - $this->config->setUserValue($id, 'settings', 'email', $mailAddress); - } + $user->setEMailAddress($mailAddress); return new DataResponse( array( diff --git a/tests/lib/contacts/localadressbook.php b/tests/lib/contacts/localadressbook.php index e5c43460835..ad3c088e3cd 100644 --- a/tests/lib/contacts/localadressbook.php +++ b/tests/lib/contacts/localadressbook.php @@ -47,6 +47,9 @@ class Test_LocalAddressBook extends \Test\TestCase class SimpleUserForTesting implements IUser { + private $uid; + private $displayName; + public function __construct($uid, $displayName) { $this->uid = $uid; @@ -105,4 +108,7 @@ class SimpleUserForTesting implements IUser { public function getCloudId() { } + + public function setEMailAddress($mailAddress) { + } } diff --git a/tests/lib/user/user.php b/tests/lib/user/user.php index 1f613edc4e6..a8d688d9c88 100644 --- a/tests/lib/user/user.php +++ b/tests/lib/user/user.php @@ -342,7 +342,8 @@ class User extends \Test\TestCase { $backend->expects($this->once()) ->method('setDisplayName') - ->with('foo','Foo'); + ->with('foo','Foo') + ->willReturn(true); $user = new \OC\User\User('foo', $backend); $this->assertTrue($user->setDisplayName('Foo')); diff --git a/tests/settings/controller/userscontrollertest.php b/tests/settings/controller/userscontrollertest.php index e1e3c4d4b6b..38fc1ee6102 100644 --- a/tests/settings/controller/userscontrollertest.php +++ b/tests/settings/controller/userscontrollertest.php @@ -1679,11 +1679,11 @@ class UsersControllerTest extends \Test\TestCase { */ public function setEmailAddressData() { return [ - /* mailAddress, isValid, expectsUpdate, expectsDelete, canChangeDisplayName, responseCode */ - [ '', true, false, true, true, Http::STATUS_OK ], - [ 'foo@local', true, true, false, true, Http::STATUS_OK], - [ 'foo@bar@local', false, false, false, true, Http::STATUS_UNPROCESSABLE_ENTITY], - [ 'foo@local', true, false, false, false, Http::STATUS_FORBIDDEN], + /* mailAddress, isValid, expectsUpdate, canChangeDisplayName, responseCode */ + [ '', true, true, true, Http::STATUS_OK ], + [ 'foo@local', true, true, true, Http::STATUS_OK], + [ 'foo@bar@local', false, false, true, Http::STATUS_UNPROCESSABLE_ENTITY], + [ 'foo@local', true, false, false, Http::STATUS_FORBIDDEN], ]; } @@ -1695,7 +1695,7 @@ class UsersControllerTest extends \Test\TestCase { * @param bool $expectsUpdate * @param bool $expectsDelete */ - public function testSetEmailAddress($mailAddress, $isValid, $expectsUpdate, $expectsDelete, $canChangeDisplayName, $responseCode) { + public function testSetEmailAddress($mailAddress, $isValid, $expectsUpdate, $canChangeDisplayName, $responseCode) { $this->container['IsAdmin'] = true; $user = $this->getMockBuilder('\OC\User\User') @@ -1708,6 +1708,13 @@ class UsersControllerTest extends \Test\TestCase { ->expects($this->any()) ->method('canChangeDisplayName') ->will($this->returnValue($canChangeDisplayName)); + $user + ->expects($expectsUpdate ? $this->once() : $this->never()) + ->method('setEMailAddress') + ->with( + $this->equalTo($mailAddress) + ); + $this->container['UserSession'] ->expects($this->atLeastOnce()) ->method('getUser') @@ -1730,26 +1737,6 @@ class UsersControllerTest extends \Test\TestCase { ->will($this->returnValue($user)); } - $this->container['Config'] - ->expects(($expectsUpdate) ? $this->once() : $this->never()) - ->method('setUserValue') - ->with( - $this->equalTo($user->getUID()), - $this->equalTo('settings'), - $this->equalTo('email'), - $this->equalTo($mailAddress) - - ); - $this->container['Config'] - ->expects(($expectsDelete) ? $this->once() : $this->never()) - ->method('deleteUserValue') - ->with( - $this->equalTo($user->getUID()), - $this->equalTo('settings'), - $this->equalTo('email') - - ); - $response = $this->container['UsersController']->setMailAddress($user->getUID(), $mailAddress); $this->assertSame($responseCode, $response->getStatus());