accounts event handler to use eventdispatcher, DI and Accounts API

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>

diff --git a/lib/base.php b/lib/base.php
index 20065006a8c7bbc2f3e7812d6b5be9680aeeaeaa..473a3419cb1436d57319f679c1b29937ab8f2732 100644 (file)
--- 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 af078bd1db099d341d8fd068d93459d13b259b23..f145fbbc3c9ae404fdb962919d00c834cd656c89 100644 (file)
--- 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 8af9e209034dfc2b6e9ca96a35ff6808457deb80..d5c7cd60b1b2dba3de603fa77821e5698cfb03ca 100644 (file)
--- 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);
-       }
 }