From 632f2d77602bf53e2eec1f192c74f31b6eba4dbe Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Mon, 27 Jun 2022 20:21:35 +0200 Subject: [PATCH] cleanup LDAP's UpdateGroups - TimedJob from API - DI of config - property types - throws hints in phpdoc - argument and return types - replace depracet execute() with executeStatement or -Query - a missing return statement Co-authored-by: Carl Schwan Signed-off-by: Arthur Schiwon --- apps/user_ldap/lib/Jobs/UpdateGroups.php | 84 ++++++++++++------------ 1 file changed, 43 insertions(+), 41 deletions(-) diff --git a/apps/user_ldap/lib/Jobs/UpdateGroups.php b/apps/user_ldap/lib/Jobs/UpdateGroups.php index b42049eb3a8..a3b306485da 100644 --- a/apps/user_ldap/lib/Jobs/UpdateGroups.php +++ b/apps/user_ldap/lib/Jobs/UpdateGroups.php @@ -28,11 +28,14 @@ */ namespace OCA\User_LDAP\Jobs; -use OC\BackgroundJob\TimedJob; +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\BackgroundJob\TimedJob; use OCA\User_LDAP\Group_Proxy; +use OCP\DB\Exception; use OCP\EventDispatcher\IEventDispatcher; use OCP\Group\Events\UserAddedEvent; use OCP\Group\Events\UserRemovedEvent; +use OCP\IConfig; use OCP\IDBConnection; use OCP\IGroupManager; use OCP\IUser; @@ -40,20 +43,14 @@ use OCP\IUserManager; use Psr\Log\LoggerInterface; class UpdateGroups extends TimedJob { - private $groupsFromDB; - - /** @var Group_Proxy */ - private $groupBackend; - /** @var IEventDispatcher */ - private $dispatcher; - /** @var IGroupManager */ - private $groupManager; - /** @var IUserManager */ - private $userManager; - /** @var LoggerInterface */ - private $logger; - /** @var IDBConnection */ - private $dbc; + /** @var ?array */ + private ?array $groupsFromDB = null; + private Group_Proxy $groupBackend; + private IEventDispatcher $dispatcher; + private IGroupManager $groupManager; + private IUserManager $userManager; + private LoggerInterface $logger; + private IDBConnection $dbc; public function __construct( Group_Proxy $groupBackend, @@ -61,9 +58,12 @@ class UpdateGroups extends TimedJob { IGroupManager $groupManager, IUserManager $userManager, LoggerInterface $logger, - IDBConnection $dbc + IDBConnection $dbc, + IConfig $config, + ITimeFactory $timeFactory ) { - $this->interval = $this->getRefreshInterval(); + parent::__construct($timeFactory); + $this->interval = (int)$config->getAppValue('user_ldap', 'bgjRefreshInterval', '3600'); $this->groupBackend = $groupBackend; $this->dispatcher = $dispatcher; $this->groupManager = $groupManager; @@ -72,27 +72,24 @@ class UpdateGroups extends TimedJob { $this->dbc = $dbc; } - /** - * @return int - */ - private function getRefreshInterval() { - //defaults to every hour - return \OC::$server->getConfig()->getAppValue('user_ldap', 'bgjRefreshInterval', 3600); - } - /** * @param mixed $argument + * @throws Exception */ - public function run($argument) { + public function run($argument): void { $this->updateGroups(); } - public function updateGroups() { + /** + * @throws Exception + */ + public function updateGroups(): void { $this->logger->debug( 'Run background job "updateGroups"', ['app' => 'user_ldap'] ); + /** @var string[] $knownGroups */ $knownGroups = array_keys($this->getKnownGroups()); $actualGroups = $this->groupBackend->getGroups(); @@ -115,17 +112,18 @@ class UpdateGroups extends TimedJob { } /** - * @return array + * @return array + * @throws Exception */ - private function getKnownGroups() { + private function getKnownGroups(): array { if (is_array($this->groupsFromDB)) { - $this->groupsFromDB; + return $this->groupsFromDB; } $qb = $this->dbc->getQueryBuilder(); $qb->select(['owncloudname', 'owncloudusers']) ->from('ldap_group_members'); - $qResult = $qb->execute(); + $qResult = $qb->executeQuery(); $result = $qResult->fetchAll(); $qResult->closeCursor(); @@ -137,7 +135,11 @@ class UpdateGroups extends TimedJob { return $this->groupsFromDB; } - private function handleKnownGroups(array $groups) { + /** + * @param string[] $groups + * @throws Exception + */ + private function handleKnownGroups(array $groups): void { $this->logger->debug( 'bgJ "updateGroups" – Dealing with known Groups.', ['app' => 'user_ldap'] @@ -147,11 +149,9 @@ class UpdateGroups extends TimedJob { ->set('owncloudusers', $qb->createParameter('members')) ->where($qb->expr()->eq('owncloudname', $qb->createParameter('groupId'))); - if (!is_array($this->groupsFromDB)) { - $this->getKnownGroups(); - } + $groupsFromDB = is_array($this->groupsFromDB) ? $this->groupsFromDB : $this->getKnownGroups(); foreach ($groups as $group) { - $knownUsers = unserialize($this->groupsFromDB[$group]['owncloudusers']); + $knownUsers = unserialize($groupsFromDB[$group]['owncloudusers']); $actualUsers = $this->groupBackend->usersInGroup($group); $hasChanged = false; @@ -191,7 +191,7 @@ class UpdateGroups extends TimedJob { 'members' => serialize($actualUsers), 'groupId' => $group ]); - $qb->execute(); + $qb->executeStatement(); } } $this->logger->debug( @@ -202,8 +202,9 @@ class UpdateGroups extends TimedJob { /** * @param string[] $createdGroups + * @throws Exception */ - private function handleCreatedGroups($createdGroups) { + private function handleCreatedGroups(array $createdGroups): void { $this->logger->debug( 'bgJ "updateGroups" – dealing with created Groups.', ['app' => 'user_ldap'] @@ -222,7 +223,7 @@ class UpdateGroups extends TimedJob { $query->setParameter('owncloudname', $createdGroup) ->setParameter('owncloudusers', $users); - $query->execute(); + $query->executeStatement(); } $this->logger->debug( 'bgJ "updateGroups" – FINISHED dealing with created Groups.', @@ -232,8 +233,9 @@ class UpdateGroups extends TimedJob { /** * @param string[] $removedGroups + * @throws Exception */ - private function handleRemovedGroups($removedGroups) { + private function handleRemovedGroups(array $removedGroups): void { $this->logger->debug( 'bgJ "updateGroups" – dealing with removed groups.', ['app' => 'user_ldap'] @@ -249,7 +251,7 @@ class UpdateGroups extends TimedJob { ['app' => 'user_ldap'] ); $query->setParameter('owncloudname', $removedGroup); - $query->execute(); + $query->executeStatement(); } $this->logger->debug( 'bgJ "updateGroups" – FINISHED dealing with removed groups.', -- 2.39.5