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 <carl@carlschwan.eu>

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

1 files 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<string, array{owncloudusers: string, owncloudname: string}>  */
+	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;
@@ -73,26 +73,23 @@ class UpdateGroups extends TimedJob {
 	}
 
 	/**
-	 * @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<string, array{owncloudusers: string, owncloudname: string}>
+	 * @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.',