From 86e5e7d9274241b0373bfa494896534b251e1978 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 22 Oct 2020 11:25:33 +0200 Subject: [PATCH] LDAP simplify User_Proxy and Group_Proxy signatures - make User_Proxy and Group_Proxy easy to instantiate - simplify dependent code - move commands to info.xml - make UpdateGroups job class non-static Signed-off-by: Arthur Schiwon --- apps/user_ldap/appinfo/info.xml | 11 ++ apps/user_ldap/appinfo/register_command.php | 61 ---------- apps/user_ldap/lib/AppInfo/Application.php | 27 ++--- apps/user_ldap/lib/Command/Search.php | 26 ++--- apps/user_ldap/lib/Group_Proxy.php | 8 +- apps/user_ldap/lib/Helper.php | 26 +---- apps/user_ldap/lib/Jobs/CleanUp.php | 13 +-- apps/user_ldap/lib/Jobs/UpdateGroups.php | 104 ++++++------------ apps/user_ldap/lib/Migration/UUIDFix.php | 7 -- apps/user_ldap/lib/Migration/UUIDFixGroup.php | 12 +- apps/user_ldap/lib/Migration/UUIDFixUser.php | 11 +- apps/user_ldap/lib/User_Proxy.php | 12 +- apps/user_ldap/tests/Jobs/CleanUpTest.php | 95 +++++++--------- .../tests/Migration/AbstractUUIDFixTest.php | 9 +- .../tests/Migration/UUIDFixGroupTest.php | 4 +- .../tests/Migration/UUIDFixUserTest.php | 2 +- apps/user_ldap/tests/User_ProxyTest.php | 6 +- psalm.xml | 1 - 18 files changed, 126 insertions(+), 309 deletions(-) delete mode 100644 apps/user_ldap/appinfo/register_command.php diff --git a/apps/user_ldap/appinfo/info.xml b/apps/user_ldap/appinfo/info.xml index cbfb0cc1c3..61197e2fd1 100644 --- a/apps/user_ldap/appinfo/info.xml +++ b/apps/user_ldap/appinfo/info.xml @@ -40,6 +40,17 @@ A user logs into Nextcloud with their LDAP or AD credentials, and is granted acc + + OCA\User_LDAP\Command\CheckUser + OCA\User_LDAP\Command\CreateEmptyConfig + OCA\User_LDAP\Command\DeleteConfig + OCA\User_LDAP\Command\Search + OCA\User_LDAP\Command\SetConfig + OCA\User_LDAP\Command\ShowConfig + OCA\User_LDAP\Command\ShowRemnants + OCA\User_LDAP\Command\TestConfig + + OCA\User_LDAP\Settings\Admin OCA\User_LDAP\Settings\Section diff --git a/apps/user_ldap/appinfo/register_command.php b/apps/user_ldap/appinfo/register_command.php deleted file mode 100644 index cdfc6fd9b1..0000000000 --- a/apps/user_ldap/appinfo/register_command.php +++ /dev/null @@ -1,61 +0,0 @@ - - * @author Joas Schilling - * @author Morris Jobke - * @author Roeland Jago Douma - * @author Roger Szabo - * @author Vinicius Cubas Brand - * - * @license AGPL-3.0 - * - * This code is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License, version 3, - * as published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License, version 3, - * along with this program. If not, see - * - */ - -use OCA\User_LDAP\Helper; -use OCA\User_LDAP\LDAP; -use OCA\User_LDAP\User_Proxy; -use OCA\User_LDAP\Mapping\UserMapping; -use OCA\User_LDAP\User\DeletedUsersIndex; - -$dbConnection = \OC::$server->getDatabaseConnection(); -$userMapping = new UserMapping($dbConnection); -$helper = new Helper(\OC::$server->getConfig()); -$ocConfig = \OC::$server->getConfig(); -$uBackend = new User_Proxy( - $helper->getServerConfigurationPrefixes(true), - new LDAP(), - $ocConfig, - \OC::$server->getNotificationManager(), - \OC::$server->getUserSession(), - \OC::$server->query(\OCA\User_LDAP\UserPluginManager::class) -); -$deletedUsersIndex = new DeletedUsersIndex( - $ocConfig, $dbConnection, $userMapping -); - -$application->add(new OCA\User_LDAP\Command\ShowConfig($helper)); -$application->add(new OCA\User_LDAP\Command\SetConfig()); -$application->add(new OCA\User_LDAP\Command\TestConfig()); -$application->add(new OCA\User_LDAP\Command\CreateEmptyConfig($helper)); -$application->add(new OCA\User_LDAP\Command\DeleteConfig($helper)); -$application->add(new OCA\User_LDAP\Command\Search($ocConfig)); -$application->add(new OCA\User_LDAP\Command\ShowRemnants( - $deletedUsersIndex, \OC::$server->getDateTimeFormatter()) -); -$application->add(new OCA\User_LDAP\Command\CheckUser( - $uBackend, $helper, $deletedUsersIndex, $userMapping) -); diff --git a/apps/user_ldap/lib/AppInfo/Application.php b/apps/user_ldap/lib/AppInfo/Application.php index 93f7ccb379..8dad63fbaf 100644 --- a/apps/user_ldap/lib/AppInfo/Application.php +++ b/apps/user_ldap/lib/AppInfo/Application.php @@ -46,11 +46,9 @@ use OCP\AppFramework\Bootstrap\IBootstrap; use OCP\AppFramework\Bootstrap\IRegistrationContext; use OCP\AppFramework\IAppContainer; use OCP\EventDispatcher\IEventDispatcher; -use OCP\IConfig; use OCP\IGroupManager; use OCP\IL10N; use OCP\IServerContainer; -use OCP\IUserSession; use OCP\Notification\IManager as INotificationManager; use Symfony\Component\EventDispatcher\EventDispatcherInterface; @@ -86,28 +84,23 @@ class Application extends App implements IBootstrap { } public function boot(IBootContext $context): void { - $context->injectFn(function (IConfig $config, - INotificationManager $notificationManager, - IUserSession $userSession, - IAppContainer $appContainer, - EventDispatcherInterface $legacyDispatcher, - IEventDispatcher $dispatcher, - IGroupManager $groupManager) { - $helper = new Helper($config); + $context->injectFn(function ( + INotificationManager $notificationManager, + IAppContainer $appContainer, + EventDispatcherInterface $legacyDispatcher, + IEventDispatcher $dispatcher, + IGroupManager $groupManager, + User_Proxy $userBackend, + Group_Proxy $groupBackend, + Helper $helper + ) { $configPrefixes = $helper->getServerConfigurationPrefixes(true); if (count($configPrefixes) > 0) { - $ldapWrapper = new LDAP(); - $notificationManager->registerNotifierService(Notifier::class); $userPluginManager = $appContainer->get(UserPluginManager::class); $groupPluginManager = $appContainer->get(GroupPluginManager::class); - $userBackend = new User_Proxy( - $configPrefixes, $ldapWrapper, $config, $notificationManager, $userSession, $userPluginManager - ); - $groupBackend = new Group_Proxy($configPrefixes, $ldapWrapper, $groupPluginManager); - \OC_User::useBackend($userBackend); $groupManager->addBackend($groupBackend); diff --git a/apps/user_ldap/lib/Command/Search.php b/apps/user_ldap/lib/Command/Search.php index 5086f3a0c1..2ee83e0f22 100644 --- a/apps/user_ldap/lib/Command/Search.php +++ b/apps/user_ldap/lib/Command/Search.php @@ -29,11 +29,9 @@ namespace OCA\User_LDAP\Command; use OCA\User_LDAP\Group_Proxy; -use OCA\User_LDAP\GroupPluginManager; use OCA\User_LDAP\Helper; use OCA\User_LDAP\LDAP; use OCA\User_LDAP\User_Proxy; -use OCA\User_LDAP\UserPluginManager; use OCP\IConfig; use Symfony\Component\Console\Command\Command; @@ -45,13 +43,16 @@ use Symfony\Component\Console\Output\OutputInterface; class Search extends Command { /** @var \OCP\IConfig */ protected $ocConfig; + /** @var User_Proxy */ + private $userProxy; + /** @var Group_Proxy */ + private $groupProxy; - /** - * @param \OCP\IConfig $ocConfig - */ - public function __construct(IConfig $ocConfig) { - $this->ocConfig = $ocConfig; + public function __construct(IConfig $ocConfig, User_Proxy $userProxy, Group_Proxy $groupProxy) { parent::__construct(); + $this->ocConfig = $ocConfig; + $this->userProxy = $userProxy; + $this->groupProxy = $groupProxy; } protected function configure() { @@ -117,7 +118,7 @@ class Search extends Command { $this->validateOffsetAndLimit($offset, $limit); if ($input->getOption('group')) { - $proxy = new Group_Proxy($configPrefixes, $ldapWrapper, \OC::$server->query(GroupPluginManager::class)); + $proxy = $this->groupProxy; $getMethod = 'getGroups'; $printID = false; // convert the limit of groups to null. This will show all the groups available instead of @@ -126,14 +127,7 @@ class Search extends Command { $limit = null; } } else { - $proxy = new User_Proxy( - $configPrefixes, - $ldapWrapper, - $this->ocConfig, - \OC::$server->getNotificationManager(), - \OC::$server->getUserSession(), - \OC::$server->query(UserPluginManager::class) - ); + $proxy = $this->userProxy; $getMethod = 'getDisplayNames'; $printID = true; } diff --git a/apps/user_ldap/lib/Group_Proxy.php b/apps/user_ldap/lib/Group_Proxy.php index 4b17c020d5..490eab4446 100644 --- a/apps/user_ldap/lib/Group_Proxy.php +++ b/apps/user_ldap/lib/Group_Proxy.php @@ -34,13 +34,9 @@ class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGet private $backends = []; private $refBackend = null; - /** - * Constructor - * - * @param string[] $serverConfigPrefixes array containing the config Prefixes - */ - public function __construct($serverConfigPrefixes, ILDAPWrapper $ldap, GroupPluginManager $groupPluginManager) { + public function __construct(Helper $helper, ILDAPWrapper $ldap, GroupPluginManager $groupPluginManager) { parent::__construct($ldap); + $serverConfigPrefixes = $helper->getServerConfigurationPrefixes(true); foreach ($serverConfigPrefixes as $configPrefix) { $this->backends[$configPrefix] = new \OCA\User_LDAP\Group_LDAP($this->getAccess($configPrefix), $groupPluginManager); diff --git a/apps/user_ldap/lib/Helper.php b/apps/user_ldap/lib/Helper.php index a8d6699814..86712534f4 100644 --- a/apps/user_ldap/lib/Helper.php +++ b/apps/user_ldap/lib/Helper.php @@ -76,7 +76,7 @@ class Helper { * except the default (first) server shall be connected to. * */ - public function getServerConfigurationPrefixes($activeConfigurations = false) { + public function getServerConfigurationPrefixes($activeConfigurations = false): array { $referenceConfigkey = 'ldap_configuration_active'; $keys = $this->getServersConfig($referenceConfigkey); @@ -188,18 +188,11 @@ class Helper { /** * checks whether there is one or more disabled LDAP configurations - * - * @return bool - * @throws \Exception */ - public function haveDisabledConfigurations() { + public function haveDisabledConfigurations(): bool { $all = $this->getServerConfigurationPrefixes(false); $active = $this->getServerConfigurationPrefixes(true); - if (!is_array($all) || !is_array($active)) { - throw new \Exception('Unexpected Return Value'); - } - return count($all) !== count($active) || count($all) === 0; } @@ -312,20 +305,7 @@ class Helper { throw new \Exception('key uid is expected to be set in $param'); } - //ain't it ironic? - $helper = new Helper(\OC::$server->getConfig()); - - $configPrefixes = $helper->getServerConfigurationPrefixes(true); - $ldapWrapper = new LDAP(); - $ocConfig = \OC::$server->getConfig(); - $notificationManager = \OC::$server->getNotificationManager(); - - $userSession = \OC::$server->getUserSession(); - $userPluginManager = \OC::$server->query(UserPluginManager::class); - - $userBackend = new User_Proxy( - $configPrefixes, $ldapWrapper, $ocConfig, $notificationManager, $userSession, $userPluginManager - ); + $userBackend = \OC::$server->get(User_Proxy::class); $uid = $userBackend->loginName2UserName($param['uid']); if ($uid !== false) { $param['uid'] = $uid; diff --git a/apps/user_ldap/lib/Jobs/CleanUp.php b/apps/user_ldap/lib/Jobs/CleanUp.php index 8f729ae437..6bb44b3e6a 100644 --- a/apps/user_ldap/lib/Jobs/CleanUp.php +++ b/apps/user_ldap/lib/Jobs/CleanUp.php @@ -35,7 +35,6 @@ use OCA\User_LDAP\Mapping\UserMapping; use OCA\User_LDAP\User\DeletedUsersIndex; use OCA\User_LDAP\User_LDAP; use OCA\User_LDAP\User_Proxy; -use OCA\User_LDAP\UserPluginManager; /** * Class CleanUp @@ -69,10 +68,11 @@ class CleanUp extends TimedJob { /** @var DeletedUsersIndex */ protected $dui; - public function __construct() { + public function __construct(User_Proxy $userBackend) { $minutes = \OC::$server->getConfig()->getSystemValue( 'ldapUserCleanupInterval', (string)$this->defaultIntervalMin); $this->setInterval((int)$minutes * 60); + $this->userBackend = $userBackend; } /** @@ -99,15 +99,6 @@ class CleanUp extends TimedJob { if (isset($arguments['userBackend'])) { $this->userBackend = $arguments['userBackend']; - } else { - $this->userBackend = new User_Proxy( - $this->ldapHelper->getServerConfigurationPrefixes(true), - new LDAP(), - $this->ocConfig, - \OC::$server->getNotificationManager(), - \OC::$server->getUserSession(), - \OC::$server->query(UserPluginManager::class) - ); } if (isset($arguments['db'])) { diff --git a/apps/user_ldap/lib/Jobs/UpdateGroups.php b/apps/user_ldap/lib/Jobs/UpdateGroups.php index 33587f8bdb..34dddb3d28 100644 --- a/apps/user_ldap/lib/Jobs/UpdateGroups.php +++ b/apps/user_ldap/lib/Jobs/UpdateGroups.php @@ -33,42 +33,36 @@ namespace OCA\User_LDAP\Jobs; -use OCA\User_LDAP\Access; -use OCA\User_LDAP\Connection; -use OCA\User_LDAP\FilesystemHelper; -use OCA\User_LDAP\GroupPluginManager; -use OCA\User_LDAP\Helper; -use OCA\User_LDAP\LDAP; -use OCA\User_LDAP\LogWrapper; -use OCA\User_LDAP\Mapping\GroupMapping; -use OCA\User_LDAP\Mapping\UserMapping; -use OCA\User_LDAP\User\Manager; +use OC\BackgroundJob\TimedJob; +use OCA\User_LDAP\Group_Proxy; use OCP\EventDispatcher\IEventDispatcher; use OCP\Group\Events\UserAddedEvent; use OCP\Group\Events\UserRemovedEvent; use OCP\ILogger; -class UpdateGroups extends \OC\BackgroundJob\TimedJob { - private static $groupsFromDB; +class UpdateGroups extends TimedJob { + private $groupsFromDB; - private static $groupBE; + /** @var Group_Proxy */ + private $groupBackend; - public function __construct() { - $this->interval = self::getRefreshInterval(); + public function __construct(Group_Proxy $groupBackend) { + $this->interval = $this->getRefreshInterval(); + $this->groupBackend = $groupBackend; } /** * @param mixed $argument */ public function run($argument) { - self::updateGroups(); + $this->updateGroups(); } - public static function updateGroups() { + public function updateGroups() { \OCP\Util::writeLog('user_ldap', 'Run background job "updateGroups"', ILogger::DEBUG); - $knownGroups = array_keys(self::getKnownGroups()); - $actualGroups = self::getGroupBE()->getGroups(); + $knownGroups = array_keys($this->getKnownGroups()); + $actualGroups = $this->groupBackend->getGroups(); if (empty($actualGroups) && empty($knownGroups)) { \OCP\Util::writeLog('user_ldap', @@ -77,9 +71,9 @@ class UpdateGroups extends \OC\BackgroundJob\TimedJob { return; } - self::handleKnownGroups(array_intersect($actualGroups, $knownGroups)); - self::handleCreatedGroups(array_diff($actualGroups, $knownGroups)); - self::handleRemovedGroups(array_diff($knownGroups, $actualGroups)); + $this->handleKnownGroups(array_intersect($actualGroups, $knownGroups)); + $this->handleCreatedGroups(array_diff($actualGroups, $knownGroups)); + $this->handleRemovedGroups(array_diff($knownGroups, $actualGroups)); \OCP\Util::writeLog('user_ldap', 'bgJ "updateGroups" – Finished.', ILogger::DEBUG); } @@ -87,7 +81,7 @@ class UpdateGroups extends \OC\BackgroundJob\TimedJob { /** * @return int */ - private static function getRefreshInterval() { + private function getRefreshInterval() { //defaults to every hour return \OC::$server->getConfig()->getAppValue('user_ldap', 'bgjRefreshInterval', 3600); } @@ -95,7 +89,7 @@ class UpdateGroups extends \OC\BackgroundJob\TimedJob { /** * @param string[] $groups */ - private static function handleKnownGroups($groups) { + private function handleKnownGroups($groups) { /** @var IEventDispatcher $dispatcher */ $dispatcher = \OC::$server->query(IEventDispatcher::class); $groupManager = \OC::$server->getGroupManager(); @@ -107,10 +101,12 @@ class UpdateGroups extends \OC\BackgroundJob\TimedJob { SET `owncloudusers` = ? WHERE `owncloudname` = ? '); + if (!is_array($this->groupsFromDB)) { + $this->getKnownGroups(); + } foreach ($groups as $group) { - //we assume, that self::$groupsFromDB has been retrieved already - $knownUsers = unserialize(self::$groupsFromDB[$group]['owncloudusers']); - $actualUsers = self::getGroupBE()->usersInGroup($group); + $knownUsers = unserialize($this->groupsFromDB[$group]['owncloudusers']); + $actualUsers = $this->groupBackend->usersInGroup($group); $hasChanged = false; $groupObject = $groupManager->get($group); @@ -142,7 +138,7 @@ class UpdateGroups extends \OC\BackgroundJob\TimedJob { /** * @param string[] $createdGroups */ - private static function handleCreatedGroups($createdGroups) { + private function handleCreatedGroups($createdGroups) { \OCP\Util::writeLog('user_ldap', 'bgJ "updateGroups" – dealing with created Groups.', ILogger::DEBUG); $query = \OC_DB::prepare(' INSERT @@ -153,7 +149,7 @@ class UpdateGroups extends \OC\BackgroundJob\TimedJob { \OCP\Util::writeLog('user_ldap', 'bgJ "updateGroups" – new group "'.$createdGroup.'" found.', ILogger::INFO); - $users = serialize(self::getGroupBE()->usersInGroup($createdGroup)); + $users = serialize($this->groupBackend->usersInGroup($createdGroup)); $query->execute([$createdGroup, $users]); } \OCP\Util::writeLog('user_ldap', @@ -164,7 +160,7 @@ class UpdateGroups extends \OC\BackgroundJob\TimedJob { /** * @param string[] $removedGroups */ - private static function handleRemovedGroups($removedGroups) { + private function handleRemovedGroups($removedGroups) { \OCP\Util::writeLog('user_ldap', 'bgJ "updateGroups" – dealing with removed groups.', ILogger::DEBUG); $query = \OC_DB::prepare(' DELETE @@ -182,59 +178,23 @@ class UpdateGroups extends \OC\BackgroundJob\TimedJob { ILogger::DEBUG); } - /** - * @return \OCA\User_LDAP\Group_LDAP|\OCA\User_LDAP\Group_Proxy - */ - private static function getGroupBE() { - if (!is_null(self::$groupBE)) { - return self::$groupBE; - } - $helper = new Helper(\OC::$server->getConfig()); - $configPrefixes = $helper->getServerConfigurationPrefixes(true); - $ldapWrapper = new LDAP(); - if (count($configPrefixes) === 1) { - //avoid the proxy when there is only one LDAP server configured - $dbc = \OC::$server->getDatabaseConnection(); - $userManager = new Manager( - \OC::$server->getConfig(), - new FilesystemHelper(), - new LogWrapper(), - \OC::$server->getAvatarManager(), - new \OCP\Image(), - $dbc, - \OC::$server->getUserManager(), - \OC::$server->getNotificationManager()); - $connector = new Connection($ldapWrapper, $configPrefixes[0]); - $ldapAccess = new Access($connector, $ldapWrapper, $userManager, $helper, \OC::$server->getConfig(), \OC::$server->getUserManager()); - $groupMapper = new GroupMapping($dbc); - $userMapper = new UserMapping($dbc); - $ldapAccess->setGroupMapper($groupMapper); - $ldapAccess->setUserMapper($userMapper); - self::$groupBE = new \OCA\User_LDAP\Group_LDAP($ldapAccess, \OC::$server->query(GroupPluginManager::class)); - } else { - self::$groupBE = new \OCA\User_LDAP\Group_Proxy($configPrefixes, $ldapWrapper, \OC::$server->query(GroupPluginManager::class)); - } - - return self::$groupBE; - } - /** * @return array */ - private static function getKnownGroups() { - if (is_array(self::$groupsFromDB)) { - return self::$groupsFromDB; + private function getKnownGroups() { + if (is_array($this->groupsFromDB)) { + $this->groupsFromDB; } $query = \OC_DB::prepare(' SELECT `owncloudname`, `owncloudusers` FROM `*PREFIX*ldap_group_members` '); $result = $query->execute()->fetchAll(); - self::$groupsFromDB = []; + $this->groupsFromDB = []; foreach ($result as $dataset) { - self::$groupsFromDB[$dataset['owncloudname']] = $dataset; + $this->groupsFromDB[$dataset['owncloudname']] = $dataset; } - return self::$groupsFromDB; + return $this->groupsFromDB; } } diff --git a/apps/user_ldap/lib/Migration/UUIDFix.php b/apps/user_ldap/lib/Migration/UUIDFix.php index 8648f97921..ba2264c3c6 100644 --- a/apps/user_ldap/lib/Migration/UUIDFix.php +++ b/apps/user_ldap/lib/Migration/UUIDFix.php @@ -50,11 +50,4 @@ abstract class UUIDFix extends QueuedJob { } } } - - /** - * @param Proxy $proxy - */ - public function overrideProxy(Proxy $proxy) { - $this->proxy = $proxy; - } } diff --git a/apps/user_ldap/lib/Migration/UUIDFixGroup.php b/apps/user_ldap/lib/Migration/UUIDFixGroup.php index b40cc8881e..2cf961f550 100644 --- a/apps/user_ldap/lib/Migration/UUIDFixGroup.php +++ b/apps/user_ldap/lib/Migration/UUIDFixGroup.php @@ -26,18 +26,12 @@ namespace OCA\User_LDAP\Migration; -use OCA\User_LDAP\Helper; -use OCA\User_LDAP\LDAP; +use OCA\User_LDAP\Group_Proxy; use OCA\User_LDAP\Mapping\GroupMapping; -use OCA\User_LDAP\User_Proxy; -use OCA\User_LDAP\UserPluginManager; -use OCP\IConfig; class UUIDFixGroup extends UUIDFix { - public function __construct(GroupMapping $mapper, LDAP $ldap, IConfig $config, Helper $helper) { + public function __construct(GroupMapping $mapper, Group_Proxy $proxy) { $this->mapper = $mapper; - $this->proxy = new User_Proxy($helper->getServerConfigurationPrefixes(true), $ldap, $config, - \OC::$server->getNotificationManager(), \OC::$server->getUserSession(), - \OC::$server->query(UserPluginManager::class)); + $this->proxy = $proxy; } } diff --git a/apps/user_ldap/lib/Migration/UUIDFixUser.php b/apps/user_ldap/lib/Migration/UUIDFixUser.php index f2be0c5ed9..4ea58c4562 100644 --- a/apps/user_ldap/lib/Migration/UUIDFixUser.php +++ b/apps/user_ldap/lib/Migration/UUIDFixUser.php @@ -26,17 +26,12 @@ namespace OCA\User_LDAP\Migration; -use OCA\User_LDAP\Group_Proxy; -use OCA\User_LDAP\GroupPluginManager; -use OCA\User_LDAP\Helper; -use OCA\User_LDAP\LDAP; +use OCA\User_LDAP\User_Proxy; use OCA\User_LDAP\Mapping\UserMapping; -use OCP\IConfig; class UUIDFixUser extends UUIDFix { - public function __construct(UserMapping $mapper, LDAP $ldap, IConfig $config, Helper $helper) { + public function __construct(UserMapping $mapper, User_Proxy $proxy) { $this->mapper = $mapper; - $groupPluginManager = \OC::$server->query(GroupPluginManager::class); - $this->proxy = new Group_Proxy($helper->getServerConfigurationPrefixes(true), $ldap, $groupPluginManager); + $this->proxy = $proxy; } } diff --git a/apps/user_ldap/lib/User_Proxy.php b/apps/user_ldap/lib/User_Proxy.php index 6445af8911..e8d0a6d694 100644 --- a/apps/user_ldap/lib/User_Proxy.php +++ b/apps/user_ldap/lib/User_Proxy.php @@ -42,17 +42,8 @@ class User_Proxy extends Proxy implements \OCP\IUserBackend, \OCP\UserInterface, /** @var User_LDAP */ private $refBackend = null; - /** - * Constructor - * - * @param array $serverConfigPrefixes array containing the config Prefixes - * @param ILDAPWrapper $ldap - * @param IConfig $ocConfig - * @param INotificationManager $notificationManager - * @param IUserSession $userSession - */ public function __construct( - array $serverConfigPrefixes, + Helper $helper, ILDAPWrapper $ldap, IConfig $ocConfig, INotificationManager $notificationManager, @@ -60,6 +51,7 @@ class User_Proxy extends Proxy implements \OCP\IUserBackend, \OCP\UserInterface, UserPluginManager $userPluginManager ) { parent::__construct($ldap); + $serverConfigPrefixes = $helper->getServerConfigurationPrefixes(true); foreach ($serverConfigPrefixes as $configPrefix) { $this->backends[$configPrefix] = new User_LDAP($this->getAccess($configPrefix), $ocConfig, $notificationManager, $userSession, $userPluginManager); diff --git a/apps/user_ldap/tests/Jobs/CleanUpTest.php b/apps/user_ldap/tests/Jobs/CleanUpTest.php index 1e88694f7b..533bb041f7 100644 --- a/apps/user_ldap/tests/Jobs/CleanUpTest.php +++ b/apps/user_ldap/tests/Jobs/CleanUpTest.php @@ -26,44 +26,49 @@ namespace OCA\User_LDAP\Tests\Jobs; +use Exception; use OCA\User_LDAP\Helper; +use OCA\User_LDAP\Jobs\CleanUp; +use OCA\User_LDAP\User\DeletedUsersIndex; +use OCA\User_LDAP\User_Proxy; use OCP\IConfig; use OCP\IDBConnection; +use Test\TestCase; -class CleanUpTest extends \Test\TestCase { - public function getMocks() { - $mocks = []; - $mocks['userBackend'] = - $this->getMockBuilder('\OCA\User_LDAP\User_Proxy') - ->disableOriginalConstructor() - ->getMock(); - $mocks['deletedUsersIndex'] = - $this->getMockBuilder('\OCA\User_LDAP\User\DeletedUsersIndex') - ->disableOriginalConstructor() - ->getMock(); - $mocks['ocConfig'] = $this->createMock(IConfig::class); - $mocks['db'] = $this->createMock(IDBConnection::class); - $mocks['helper'] = $this->createMock(Helper::class); +class CleanUpTest extends TestCase { + /** @var CleanUp */ + protected $bgJob; - return $mocks; + /** @var array */ + protected $mocks; + + public function setUp(): void { + $this->createMocks(); + $this->bgJob = new CleanUp($this->mocks['userBackend']); + $this->bgJob->setArguments($this->mocks); + } + + protected function createMocks(): void { + $this->mocks = []; + $this->mocks['userBackend'] = $this->createMock(User_Proxy::class); + $this->mocks['deletedUsersIndex'] = $this->createMock(DeletedUsersIndex::class); + $this->mocks['ocConfig'] = $this->createMock(IConfig::class); + $this->mocks['db'] = $this->createMock(IDBConnection::class); + $this->mocks['helper'] = $this->createMock(Helper::class); } /** * clean up job must not run when there are disabled configurations */ public function test_runNotAllowedByDisabledConfigurations() { - $args = $this->getMocks(); - $args['helper']->expects($this->once()) + $this->mocks['helper']->expects($this->once()) ->method('haveDisabledConfigurations') ->willReturn(true); - $args['ocConfig']->expects($this->never()) + $this->mocks['ocConfig']->expects($this->never()) ->method('getSystemValue'); - $bgJob = new \OCA\User_LDAP\Jobs\CleanUp(); - $bgJob->setArguments($args); - - $result = $bgJob->isCleanUpAllowed(); + $result = $this->bgJob->isCleanUpAllowed(); $this->assertSame(false, $result); } @@ -72,18 +77,14 @@ class CleanUpTest extends \Test\TestCase { * returning unexpected results */ public function test_runNotAllowedByBrokenHelper() { - $args = $this->getMocks(); - $args['helper']->expects($this->once()) + $this->mocks['helper']->expects($this->once()) ->method('haveDisabledConfigurations') - ->will($this->throwException(new \Exception())); + ->will($this->throwException(new Exception())); - $args['ocConfig']->expects($this->never()) + $this->mocks['ocConfig']->expects($this->never()) ->method('getSystemValue'); - $bgJob = new \OCA\User_LDAP\Jobs\CleanUp(); - $bgJob->setArguments($args); - - $result = $bgJob->isCleanUpAllowed(); + $result = $this->bgJob->isCleanUpAllowed(); $this->assertSame(false, $result); } @@ -91,19 +92,15 @@ class CleanUpTest extends \Test\TestCase { * clean up job must not run when it is not enabled */ public function test_runNotAllowedBySysConfig() { - $args = $this->getMocks(); - $args['helper']->expects($this->once()) + $this->mocks['helper']->expects($this->once()) ->method('haveDisabledConfigurations') ->willReturn(false); - $args['ocConfig']->expects($this->once()) + $this->mocks['ocConfig']->expects($this->once()) ->method('getSystemValue') ->willReturn(false); - $bgJob = new \OCA\User_LDAP\Jobs\CleanUp(); - $bgJob->setArguments($args); - - $result = $bgJob->isCleanUpAllowed(); + $result = $this->bgJob->isCleanUpAllowed(); $this->assertSame(false, $result); } @@ -111,19 +108,15 @@ class CleanUpTest extends \Test\TestCase { * clean up job is allowed to run */ public function test_runIsAllowed() { - $args = $this->getMocks(); - $args['helper']->expects($this->once()) + $this->mocks['helper']->expects($this->once()) ->method('haveDisabledConfigurations') ->willReturn(false); - $args['ocConfig']->expects($this->once()) + $this->mocks['ocConfig']->expects($this->once()) ->method('getSystemValue') ->willReturn(true); - $bgJob = new \OCA\User_LDAP\Jobs\CleanUp(); - $bgJob->setArguments($args); - - $result = $bgJob->isCleanUpAllowed(); + $result = $this->bgJob->isCleanUpAllowed(); $this->assertSame(true, $result); } @@ -131,12 +124,7 @@ class CleanUpTest extends \Test\TestCase { * check whether offset will be reset when it needs to */ public function test_OffsetResetIsNecessary() { - $args = $this->getMocks(); - - $bgJob = new \OCA\User_LDAP\Jobs\CleanUp(); - $bgJob->setArguments($args); - - $result = $bgJob->isOffsetResetNecessary($bgJob->getChunkSize() - 1); + $result = $this->bgJob->isOffsetResetNecessary($this->bgJob->getChunkSize() - 1); $this->assertSame(true, $result); } @@ -144,12 +132,7 @@ class CleanUpTest extends \Test\TestCase { * make sure offset is not reset when it is not due */ public function test_OffsetResetIsNotNecessary() { - $args = $this->getMocks(); - - $bgJob = new \OCA\User_LDAP\Jobs\CleanUp(); - $bgJob->setArguments($args); - - $result = $bgJob->isOffsetResetNecessary($bgJob->getChunkSize()); + $result = $this->bgJob->isOffsetResetNecessary($this->bgJob->getChunkSize()); $this->assertSame(false, $result); } } diff --git a/apps/user_ldap/tests/Migration/AbstractUUIDFixTest.php b/apps/user_ldap/tests/Migration/AbstractUUIDFixTest.php index ec484dfe7e..82483a5432 100644 --- a/apps/user_ldap/tests/Migration/AbstractUUIDFixTest.php +++ b/apps/user_ldap/tests/Migration/AbstractUUIDFixTest.php @@ -74,18 +74,13 @@ abstract class AbstractUUIDFixTest extends TestCase { ->willReturn(['s01', 's03']); } - protected function mockProxy($className) { - $this->proxy = $this->createMock($className); + protected function instantiateJob($className) { + $this->job = new $className($this->mapper, $this->proxy); $this->proxy->expects($this->any()) ->method('getLDAPAccess') ->willReturn($this->access); } - protected function instantiateJob($className) { - $this->job = new $className($this->mapper, $this->ldap, $this->config, $this->helper); - $this->job->overrideProxy($this->proxy); - } - public function testRunSingleRecord() { $args = [ 'records' => [ diff --git a/apps/user_ldap/tests/Migration/UUIDFixGroupTest.php b/apps/user_ldap/tests/Migration/UUIDFixGroupTest.php index a1f04d4467..fb9d2e2331 100644 --- a/apps/user_ldap/tests/Migration/UUIDFixGroupTest.php +++ b/apps/user_ldap/tests/Migration/UUIDFixGroupTest.php @@ -40,11 +40,9 @@ class UUIDFixGroupTest extends AbstractUUIDFixTest { $this->isUser = false; parent::setUp(); - $this->isUser = false; - $this->mapper = $this->createMock(GroupMapping::class); + $this->proxy = $this->createMock(Group_Proxy::class); - $this->mockProxy(Group_Proxy::class); $this->instantiateJob(UUIDFixGroup::class); } } diff --git a/apps/user_ldap/tests/Migration/UUIDFixUserTest.php b/apps/user_ldap/tests/Migration/UUIDFixUserTest.php index 188c7b0f91..cf80bf3623 100644 --- a/apps/user_ldap/tests/Migration/UUIDFixUserTest.php +++ b/apps/user_ldap/tests/Migration/UUIDFixUserTest.php @@ -40,8 +40,8 @@ class UUIDFixUserTest extends AbstractUUIDFixTest { parent::setUp(); $this->mapper = $this->createMock(UserMapping::class); + $this->proxy = $this->createMock(User_Proxy::class); - $this->mockProxy(User_Proxy::class); $this->instantiateJob(UUIDFixUser::class); } } diff --git a/apps/user_ldap/tests/User_ProxyTest.php b/apps/user_ldap/tests/User_ProxyTest.php index 5753990a73..0bbea00ed4 100644 --- a/apps/user_ldap/tests/User_ProxyTest.php +++ b/apps/user_ldap/tests/User_ProxyTest.php @@ -29,6 +29,7 @@ namespace OCA\User_LDAP\Tests; +use OCA\User_LDAP\Helper; use OCA\User_LDAP\ILDAPWrapper; use OCA\User_LDAP\User_Proxy; use OCA\User_LDAP\UserPluginManager; @@ -38,6 +39,8 @@ use OCP\Notification\IManager as INotificationManager; use Test\TestCase; class User_ProxyTest extends TestCase { + /** @var Helper|\PHPUnit\Framework\MockObject\MockObject */ + protected $helper; /** @var ILDAPWrapper|\PHPUnit\Framework\MockObject\MockObject */ private $ldapWrapper; /** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */ @@ -54,6 +57,7 @@ class User_ProxyTest extends TestCase { protected function setUp(): void { parent::setUp(); + $this->helper = $this->createMock(Helper::class); $this->ldapWrapper = $this->createMock(ILDAPWrapper::class); $this->config = $this->createMock(IConfig::class); $this->notificationManager = $this->createMock(INotificationManager::class); @@ -61,7 +65,7 @@ class User_ProxyTest extends TestCase { $this->userPluginManager = $this->createMock(UserPluginManager::class); $this->proxy = $this->getMockBuilder(User_Proxy::class) ->setConstructorArgs([ - [], + $this->helper, $this->ldapWrapper, $this->config, $this->notificationManager, diff --git a/psalm.xml b/psalm.xml index f299558450..4e3bced149 100644 --- a/psalm.xml +++ b/psalm.xml @@ -79,7 +79,6 @@ -