Refactor user_ldap group membership to use flat DB

Move away from serialized arrays. Also use a QBMapper class for the new table.

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>

5 files changed, 308 insertions, 90 deletions

diff --git a/apps/user_ldap/lib/Db/GroupMembership.php b/apps/user_ldap/lib/Db/GroupMembership.php
new file mode 100644
index 00000000000..25be5296d52
--- /dev/null
+++ b/apps/user_ldap/lib/Db/GroupMembership.php
@@ -0,0 +1,48 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2023 Côme Chilliet <come.chilliet@nextcloud.com>
+ *
+ * @author Côme Chilliet <come.chilliet@nextcloud.com>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * 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
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCA\User_LDAP\Db;
+
+use OCP\AppFramework\Db\Entity;
+
+/**
+ * @method void setUserid(string $userid)
+ * @method string getUserid()
+ * @method void setGroupid(string $groupid)
+ * @method string getGroupid()
+ */
+class GroupMembership extends Entity {
+	/** @var string */
+	protected $groupid;
+
+	/** @var string */
+	protected $userid;
+
+	public function __construct() {
+		$this->addType('groupid', 'string');
+		$this->addType('userid', 'string');
+	}
+}
diff --git a/apps/user_ldap/lib/Db/GroupMembershipMapper.php b/apps/user_ldap/lib/Db/GroupMembershipMapper.php
new file mode 100644
index 00000000000..2c30e947cb0
--- /dev/null
+++ b/apps/user_ldap/lib/Db/GroupMembershipMapper.php
@@ -0,0 +1,77 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2023 Côme Chilliet <come.chilliet@nextcloud.com>
+ *
+ * @author Côme Chilliet <come.chilliet@nextcloud.com>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * 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
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCA\User_LDAP\Db;
+
+use OCP\AppFramework\Db\QBMapper;
+use OCP\DB\QueryBuilder\IQueryBuilder;
+use OCP\IDBConnection;
+
+/**
+ * @template-extends QBMapper<GroupMembership>
+ */
+class GroupMembershipMapper extends QBMapper {
+	public function __construct(IDBConnection $db) {
+		parent::__construct($db, 'ldap_group_membership', GroupMembership::class);
+	}
+
+	/**
+	 * @return string[]
+	 */
+	public function getKnownGroups(): array {
+		$query = $this->db->getQueryBuilder();
+		$result = $query->selectDistinct('groupid')
+			->from($this->getTableName())
+			->executeQuery();
+
+		$groups = $result->fetchAll();
+		$result->closeCursor();
+		return $groups;
+	}
+
+	/**
+	 * @return GroupMembership[]
+	 */
+	public function findGroupMemberships(string $groupid): array {
+		$qb = $this->db->getQueryBuilder();
+		$select = $qb->select('*')
+			->from($this->getTableName())
+			->where($qb->expr()->eq('groupid', $qb->createNamedParameter($groupid)));
+
+		return $this->findEntities($select);
+	}
+
+	public function deleteGroups(array $removedGroups): void {
+		$query = $this->db->getQueryBuilder();
+		$query->delete($this->getTableName())
+			->where($query->expr()->in('groupid', $query->createParameter('groupids')));
+
+		foreach (array_chunk($removedGroups, 1000) as $removedGroupsChunk) {
+			$query->setParameter('groupids', $removedGroupsChunk, IQueryBuilder::PARAM_STR_ARRAY);
+			$query->executeStatement();
+		}
+	}
+}
diff --git a/apps/user_ldap/lib/Jobs/UpdateGroups.php b/apps/user_ldap/lib/Jobs/UpdateGroups.php
index 48687494a61..2c3ce457cff 100644
--- a/apps/user_ldap/lib/Jobs/UpdateGroups.php
+++ b/apps/user_ldap/lib/Jobs/UpdateGroups.php
@@ -30,9 +30,10 @@ namespace OCA\User_LDAP\Jobs;
 
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\TimedJob;
+use OCP\User_LDAP\Db\GroupMembership;
+use OCP\User_LDAP\Db\GroupMembershipMapper;
 use OCA\User_LDAP\Group_Proxy;
 use OCP\DB\Exception;
-use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Group\Events\UserAddedEvent;
 use OCP\Group\Events\UserRemovedEvent;
@@ -46,31 +47,20 @@ use Psr\Log\LoggerInterface;
 class UpdateGroups extends TimedJob {
 	/** @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,
-		IEventDispatcher $dispatcher,
-		IGroupManager $groupManager,
-		IUserManager $userManager,
-		LoggerInterface $logger,
-		IDBConnection $dbc,
+		private Group_Proxy $groupBackend,
+		private IEventDispatcher $dispatcher,
+		private IGroupManager $groupManager,
+		private IUserManager $userManager,
+		private LoggerInterface $logger,
+		private IDBConnection $dbc,
+		private GroupMembershipMapper $groupMembershipMapper,
 		IConfig $config,
-		ITimeFactory $timeFactory
+		ITimeFactory $timeFactory,
 	) {
 		parent::__construct($timeFactory);
 		$this->interval = (int)$config->getAppValue('user_ldap', 'bgjRefreshInterval', '3600');
-		$this->groupBackend = $groupBackend;
-		$this->dispatcher = $dispatcher;
-		$this->groupManager = $groupManager;
-		$this->userManager = $userManager;
-		$this->logger = $logger;
-		$this->dbc = $dbc;
 	}
 
 	/**
@@ -90,8 +80,7 @@ class UpdateGroups extends TimedJob {
 			['app' => 'user_ldap']
 		);
 
-		/** @var string[] $knownGroups */
-		$knownGroups = array_keys($this->getKnownGroups());
+		$knownGroups = $this->groupMembershipMapper->getKnownGroups();
 		$actualGroups = $this->groupBackend->getGroups();
 
 		if (empty($actualGroups) && empty($knownGroups)) {
@@ -113,30 +102,6 @@ class UpdateGroups extends TimedJob {
 	}
 
 	/**
-	 * @return array<string, array{owncloudusers: string, owncloudname: string}>
-	 * @throws Exception
-	 */
-	private function getKnownGroups(): array {
-		if (is_array($this->groupsFromDB)) {
-			return $this->groupsFromDB;
-		}
-		$qb = $this->dbc->getQueryBuilder();
-		$qb->select(['owncloudname', 'owncloudusers'])
-			->from('ldap_group_members');
-
-		$qResult = $qb->executeQuery();
-		$result = $qResult->fetchAll();
-		$qResult->closeCursor();
-
-		$this->groupsFromDB = [];
-		foreach ($result as $dataset) {
-			$this->groupsFromDB[$dataset['owncloudname']] = $dataset;
-		}
-
-		return $this->groupsFromDB;
-	}
-
-	/**
 	 * @param string[] $groups
 	 * @throws Exception
 	 */
@@ -145,16 +110,15 @@ class UpdateGroups extends TimedJob {
 			'bgJ "updateGroups" – Dealing with known Groups.',
 			['app' => 'user_ldap']
 		);
-		$qb = $this->dbc->getQueryBuilder();
-		$qb->update('ldap_group_members')
-			->set('owncloudusers', $qb->createParameter('members'))
-			->where($qb->expr()->eq('owncloudname', $qb->createParameter('groupId')));
 
-		$groupsFromDB = $this->getKnownGroups();
 		foreach ($groups as $group) {
-			$knownUsers = unserialize($groupsFromDB[$group]['owncloudusers']);
+			$groupMemberships = $this->groupMembershipMapper->findGroupMemberships($group);
+			$knownUsers = array_map(
+				fn (GroupMembership $groupMembership): string => $groupMembership->getUserid(),
+				$groupMemberships
+			);
+			$groupMemberships = array_combine($knownUsers, $groupMemberships);
 			$actualUsers = $this->groupBackend->usersInGroup($group);
-			$hasChanged = false;
 
 			$groupObject = $this->groupManager->get($group);
 			if ($groupObject === null) {
@@ -169,6 +133,7 @@ class UpdateGroups extends TimedJob {
 				continue;
 			}
 			foreach (array_diff($knownUsers, $actualUsers) as $removedUser) {
+				$this->groupMembershipMapper->delete($groupMemberships[$removedUser]);
 				$userObject = $this->userManager->get($removedUser);
 				if ($userObject instanceof IUser) {
 					$this->dispatcher->dispatchTyped(new UserRemovedEvent($groupObject, $userObject));
@@ -181,9 +146,9 @@ class UpdateGroups extends TimedJob {
 						'group' => $group
 					]
 				);
-				$hasChanged = true;
 			}
 			foreach (array_diff($actualUsers, $knownUsers) as $addedUser) {
+				$this->groupMembershipMapper->insert(GroupMembership::fromParams(['groupid' => $group,'userid' => $addedUser]));
 				$userObject = $this->userManager->get($addedUser);
 				if ($userObject instanceof IUser) {
 					$this->dispatcher->dispatchTyped(new UserAddedEvent($groupObject, $userObject));
@@ -196,14 +161,6 @@ class UpdateGroups extends TimedJob {
 						'group' => $group
 					]
 				);
-				$hasChanged = true;
-			}
-			if ($hasChanged) {
-				$qb->setParameters([
-					'members' => serialize($actualUsers),
-					'groupId' => $group
-				]);
-				$qb->executeStatement();
 			}
 		}
 		$this->logger->debug(
@@ -222,21 +179,16 @@ class UpdateGroups extends TimedJob {
 			['app' => 'user_ldap']
 		);
 
-		$query = $this->dbc->getQueryBuilder();
-		$query->insert('ldap_group_members')
-			->setValue('owncloudname', $query->createParameter('owncloudname'))
-			->setValue('owncloudusers', $query->createParameter('owncloudusers'));
-
 		foreach ($createdGroups as $createdGroup) {
 			$this->logger->info(
 				'bgJ "updateGroups" – new group "' . $createdGroup . '" found.',
 				['app' => 'user_ldap']
 			);
-			$users = serialize($this->groupBackend->usersInGroup($createdGroup));
 
-			$query->setParameter('owncloudname', $createdGroup)
-				->setParameter('owncloudusers', $users);
-			$query->executeStatement();
+			$users = $this->groupBackend->usersInGroup($createdGroup);
+			foreach ($users as $user) {
+				$this->groupMembershipMapper->insert(GroupMembership::fromParams(['groupid' => $createdGroup,'userid' => $user]));
+			}
 		}
 		$this->logger->debug(
 			'bgJ "updateGroups" – FINISHED dealing with created Groups.',
@@ -254,25 +206,13 @@ class UpdateGroups extends TimedJob {
 			['app' => 'user_ldap']
 		);
 
-		$query = $this->dbc->getQueryBuilder();
-		$query->delete('ldap_group_members')
-			->where($query->expr()->in('owncloudname', $query->createParameter('owncloudnames')));
-
-		foreach (array_chunk($removedGroups, 1000) as $removedGroupsChunk) {
-			$this->logger->info(
-				'bgJ "updateGroups" – groups {removedGroups} were removed.',
-				[
-					'app' => 'user_ldap',
-					'removedGroups' => $removedGroupsChunk
-				]
-			);
-			$query->setParameter('owncloudnames', $removedGroupsChunk, IQueryBuilder::PARAM_STR_ARRAY);
-			$query->executeStatement();
-		}
-
-		$this->logger->debug(
-			'bgJ "updateGroups" – FINISHED dealing with removed groups.',
-			['app' => 'user_ldap']
+		$this->groupMembershipMapper->deleteGroups($removedGroups);
+		$this->logger->info(
+			'bgJ "updateGroups" – groups {removedGroups} were removed.',
+			[
+				'app' => 'user_ldap',
+				'removedGroups' => $removedGroups
+			]
 		);
 	}
 }
diff --git a/apps/user_ldap/lib/Migration/Version1190Date20230706134108.php b/apps/user_ldap/lib/Migration/Version1190Date20230706134108.php
new file mode 100644
index 00000000000..cf41ba1ec93
--- /dev/null
+++ b/apps/user_ldap/lib/Migration/Version1190Date20230706134108.php
@@ -0,0 +1,107 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2023 Your name <your@email.com>
+ *
+ * @author Your name <your@email.com>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * 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
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCA\User_LDAP\Migration;
+
+use Closure;
+use OCP\DB\ISchemaWrapper;
+use OCP\DB\Types;
+use OCP\IDBConnection;
+use OCP\Migration\IOutput;
+use OCP\Migration\SimpleMigrationStep;
+
+class Version1190Date20230706134108 extends SimpleMigrationStep {
+	public function __construct(
+		private IDBConnection $dbc,
+	) {
+	}
+
+	public function preSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void {
+	}
+
+	public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
+		/** @var ISchemaWrapper $schema */
+		$schema = $schemaClosure();
+
+		if (!$schema->hasTable('ldap_group_membership')) {
+			$table = $schema->createTable('ldap_group_membership');
+			$table->addColumn('groupid', Types::STRING, [
+				'notnull' => true,
+				'length' => 255,
+				'default' => '',
+			]);
+			$table->addColumn('userid', Types::STRING, [
+				'notnull' => true,
+				'length' => 64,
+				'default' => '',
+			]);
+			return $schema;
+		} else {
+			return null;
+		}
+	}
+
+	public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void {
+		/** @var ISchemaWrapper $schema */
+		$schema = $schemaClosure();
+
+		if (!$schema->hasTable('ldap_group_members')) {
+			// Old table does not exist
+			return;
+		}
+
+		$output->startProgress();
+		$this->copyGroupMembershipData();
+		$output->finishProgress();
+	}
+
+	protected function copyGroupMembershipData(): void {
+		$insert = $this->dbc->getQueryBuilder();
+		$insert->insert('ldap_group_membership')
+			->values([
+				'userid' => $insert->createParameter('userid'),
+				'groupid' => $insert->createParameter('groupid'),
+			]);
+
+		$query = $this->dbc->getQueryBuilder();
+		$query->select('*')
+			->from('ldap_group_members');
+
+		$result = $query->executeQuery();
+		while ($row = $result->fetch()) {
+			$knownUsers = unserialize($row['owncloudusers']);
+			foreach ($knownUsers as $knownUser) {
+				$insert
+					->setParameter('groupid', $row['owncloudname'])
+					->setParameter('userid', $knownUser)
+				;
+
+				$insert->executeStatement();
+			}
+		}
+		$result->closeCursor();
+	}
+}
diff --git a/apps/user_ldap/lib/Migration/Version1190Date20230706134109.php b/apps/user_ldap/lib/Migration/Version1190Date20230706134109.php
new file mode 100644
index 00000000000..bc88dc2beb9
--- /dev/null
+++ b/apps/user_ldap/lib/Migration/Version1190Date20230706134109.php
@@ -0,0 +1,46 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2023 Your name <your@email.com>
+ *
+ * @author Your name <your@email.com>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * 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
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCA\User_LDAP\Migration;
+
+use Closure;
+use OCP\DB\ISchemaWrapper;
+use OCP\Migration\IOutput;
+use OCP\Migration\SimpleMigrationStep;
+
+class Version1190Date20230706134109 extends SimpleMigrationStep {
+	public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
+		/** @var ISchemaWrapper $schema */
+		$schema = $schemaClosure();
+
+		if ($schema->hasTable('ldap_group_members')) {
+			$schema->dropTable('ldap_group_members');
+			return $schema;
+		}
+
+		return null;
+	}
+}