From 51cae9ba98d52a27bad5aea8f33af53721c011f2 Mon Sep 17 00:00:00 2001
From: Arthur Schiwon <blizzz@arthur-schiwon.de>
Date: Tue, 15 Jun 2021 19:32:39 +0200
Subject: accounts event handler to use eventdispatcher, DI and Accounts API
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
---
lib/base.php | 6 ++-
lib/private/Accounts/Hooks.php | 67 ++++++++++++++------------------
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);
- }
}
--
cgit v1.2.3