diff options
-rw-r--r-- | lib/base.php | 6 | ||||
-rw-r--r-- | lib/private/Accounts/Hooks.php | 67 | ||||
-rw-r--r-- | tests/lib/Accounts/HooksTest.php | 84 |
3 files changed, 74 insertions, 83 deletions
diff --git a/lib/base.php b/lib/base.php index 20065006a8c..473a3419cb1 100644 --- a/lib/base.php +++ b/lib/base.php @@ -68,6 +68,7 @@ use OCP\Share; use OC\Encryption\HookManager; use OC\Files\Filesystem; use OC\Share20\Hooks; +use OCP\User\Events\UserChangedEvent; require_once 'public/Constants.php'; @@ -843,8 +844,9 @@ class OC { } private static function registerAccountHooks() { - $hookHandler = \OC::$server->get(\OC\Accounts\Hooks::class); - \OCP\Util::connectHook('OC_User', 'changeUser', $hookHandler, 'changeUserHook'); + /** @var IEventDispatcher $dispatcher */ + $dispatcher = \OC::$server->get(IEventDispatcher::class); + $dispatcher->addServiceListener(UserChangedEvent::class, \OC\Accounts\Hooks::class); } private static function registerAppRestrictionsHooks() { diff --git a/lib/private/Accounts/Hooks.php b/lib/private/Accounts/Hooks.php index af078bd1db0..f145fbbc3c9 100644 --- a/lib/private/Accounts/Hooks.php +++ b/lib/private/Accounts/Hooks.php @@ -25,66 +25,55 @@ namespace OC\Accounts; use OCP\Accounts\IAccountManager; +use OCP\Accounts\PropertyDoesNotExistException; +use OCP\EventDispatcher\Event; +use OCP\EventDispatcher\IEventListener; use OCP\IUser; +use OCP\User\Events\UserChangedEvent; use Psr\Log\LoggerInterface; -class Hooks { +class Hooks implements IEventListener { - /** @var AccountManager|null */ + /** @var IAccountManager */ private $accountManager; - /** @var LoggerInterface */ private $logger; - public function __construct(LoggerInterface $logger) { + public function __construct(LoggerInterface $logger, IAccountManager $accountManager) { $this->logger = $logger; + $this->accountManager = $accountManager; } /** * update accounts table if email address or display name was changed from outside - * - * @param array $params */ - public function changeUserHook($params) { - $accountManager = $this->getAccountManager(); - - /** @var IUser $user */ - $user = isset($params['user']) ? $params['user'] : null; - $feature = isset($params['feature']) ? $params['feature'] : null; - $newValue = isset($params['value']) ? $params['value'] : null; + public function changeUserHook(IUser $user, string $feature, $newValue): void { + $account = $this->accountManager->getAccount($user); - if (is_null($user) || is_null($feature) || is_null($newValue)) { - $this->logger->warning('Missing expected parameters in change user hook'); + try { + switch ($feature) { + case 'eMailAddress': + $property = $account->getProperty(IAccountManager::PROPERTY_EMAIL); + break; + case 'displayName': + $property = $account->getProperty(IAccountManager::PROPERTY_DISPLAYNAME); + break; + } + } catch (PropertyDoesNotExistException $e) { + $this->logger->debug($e->getMessage(), ['exception' => $e]); return; } - $accountData = $accountManager->getUser($user); - - switch ($feature) { - case 'eMailAddress': - if ($accountData[IAccountManager::PROPERTY_EMAIL]['value'] !== $newValue) { - $accountData[IAccountManager::PROPERTY_EMAIL]['value'] = $newValue; - $accountManager->updateUser($user, $accountData); - } - break; - case 'displayName': - if ($accountData[IAccountManager::PROPERTY_DISPLAYNAME]['value'] !== $newValue) { - $accountData[IAccountManager::PROPERTY_DISPLAYNAME]['value'] = $newValue; - $accountManager->updateUser($user, $accountData); - } - break; + if (isset($property) && $property->getValue() !== (string)$newValue) { + $property->setValue($newValue); + $this->accountManager->updateAccount($account); } } - /** - * return instance of accountManager - * - * @return AccountManager - */ - protected function getAccountManager(): AccountManager { - if ($this->accountManager === null) { - $this->accountManager = \OC::$server->query(AccountManager::class); + public function handle(Event $event): void { + if(!$event instanceof UserChangedEvent) { + return; } - return $this->accountManager; + $this->changeUserHook($event->getUser(), $event->getFeature(), $event->getValue()); } } diff --git a/tests/lib/Accounts/HooksTest.php b/tests/lib/Accounts/HooksTest.php index 8af9e209034..d5c7cd60b1b 100644 --- a/tests/lib/Accounts/HooksTest.php +++ b/tests/lib/Accounts/HooksTest.php @@ -23,7 +23,9 @@ namespace Test\Accounts; use OC\Accounts\AccountManager; use OC\Accounts\Hooks; +use OCP\Accounts\IAccount; use OCP\Accounts\IAccountManager; +use OCP\Accounts\IAccountProperty; use OCP\IUser; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; @@ -43,7 +45,7 @@ class HooksTest extends TestCase { /** @var AccountManager|MockObject */ private $accountManager; - /** @var Hooks|MockObject */ + /** @var Hooks */ private $hooks; protected function setUp(): void { @@ -53,12 +55,7 @@ class HooksTest extends TestCase { $this->accountManager = $this->getMockBuilder(AccountManager::class) ->disableOriginalConstructor()->getMock(); - $this->hooks = $this->getMockBuilder(Hooks::class) - ->setConstructorArgs([$this->logger]) - ->setMethods(['getAccountManager']) - ->getMock(); - - $this->hooks->method('getAccountManager')->willReturn($this->accountManager); + $this->hooks = new Hooks($this->logger, $this->accountManager); } /** @@ -72,48 +69,57 @@ class HooksTest extends TestCase { */ public function testChangeUserHook($params, $data, $setEmail, $setDisplayName, $error) { if ($error) { - $this->accountManager->expects($this->never())->method('getUser'); - $this->accountManager->expects($this->never())->method('updateUser'); + $this->accountManager->expects($this->never())->method('updateAccount'); } else { - $this->accountManager->expects($this->once())->method('getUser')->willReturn($data); - $newData = $data; + $account = $this->createMock(IAccount::class); + $this->accountManager->expects($this->atLeastOnce())->method('getAccount')->willReturn($account); if ($setEmail) { - $newData[IAccountManager::PROPERTY_EMAIL]['value'] = $params['value']; - $this->accountManager->expects($this->once())->method('updateUser') - ->with($params['user'], $newData); + $property = $this->createMock(IAccountProperty::class); + $property->expects($this->atLeastOnce()) + ->method('getValue') + ->willReturn($data[IAccountManager::PROPERTY_EMAIL]['value']); + $property->expects($this->atLeastOnce()) + ->method('setValue') + ->with($params['value']); + + $account->expects($this->atLeastOnce()) + ->method('getProperty') + ->with(IAccountManager::PROPERTY_EMAIL) + ->willReturn($property); + + $this->accountManager->expects($this->once()) + ->method('updateAccount') + ->with($account); } elseif ($setDisplayName) { - $newData[IAccountManager::PROPERTY_DISPLAYNAME]['value'] = $params['value']; - $this->accountManager->expects($this->once())->method('updateUser') - ->with($params['user'], $newData); + $property = $this->createMock(IAccountProperty::class); + $property->expects($this->atLeastOnce()) + ->method('getValue') + ->willReturn($data[IAccountManager::PROPERTY_DISPLAYNAME]['value']); + $property->expects($this->atLeastOnce()) + ->method('setValue') + ->with($params['value']); + + $account->expects($this->atLeastOnce()) + ->method('getProperty') + ->with(IAccountManager::PROPERTY_DISPLAYNAME) + ->willReturn($property); + + $this->accountManager->expects($this->once()) + ->method('updateAccount') + ->with($account); } else { - $this->accountManager->expects($this->never())->method('updateUser'); + $this->accountManager->expects($this->never())->method('updateAccount'); } } - $this->hooks->changeUserHook($params); + $this->hooks->changeUserHook($params['user'], $params['feature'], $params['value']); } public function dataTestChangeUserHook() { $user = $this->createMock(IUser::class); return [ [ - ['feature' => '', 'value' => ''], - [ - IAccountManager::PROPERTY_EMAIL => ['value' => ''], - IAccountManager::PROPERTY_DISPLAYNAME => ['value' => ''] - ], - false, false, true - ], - [ - ['user' => $user, 'value' => ''], - [ - IAccountManager::PROPERTY_EMAIL => ['value' => ''], - IAccountManager::PROPERTY_DISPLAYNAME => ['value' => ''] - ], - false, false, true - ], - [ - ['user' => $user, 'feature' => ''], + ['user' => $user, 'feature' => '', 'value' => ''], [ IAccountManager::PROPERTY_EMAIL => ['value' => ''], IAccountManager::PROPERTY_DISPLAYNAME => ['value' => ''] @@ -146,10 +152,4 @@ class HooksTest extends TestCase { ], ]; } - - public function testGetAccountManager() { - $hooks = new Hooks($this->logger); - $result = $this->invokePrivate($hooks, 'getAccountManager'); - $this->assertInstanceOf(AccountManager::class, $result); - } } |