Merge pull request #19240 from nextcloud/bugfix/noid/share-cleanup

fix(sharing): Add a repair step to remove orphan entries

Author: nickvergessen

appinfo/info.xml

@@ -86,6 +86,7 @@
 			<step>OCA\Talk\Migration\FixNamespaceInDatabaseTables</step>
 		</pre-migration>
 		<post-migration>
+			<step>OCA\Talk\Migration\CleanupOldShares</step>
 			<step>OCA\Talk\Migration\ClearResourceAccessCache</step>
 			<step>OCA\Talk\Migration\CacheUserDisplayNames</step>
 			<step>OCA\Talk\Migration\FixLastReadMessageZero</step>

lib/AppInfo/Application.php

@@ -274,6 +274,7 @@ public function register(IRegistrationContext $context): void {
 		// Sharing listeners
 		$context->registerEventListener(BeforeShareCreatedEvent::class, ShareListener::class, 1000);
 		$context->registerEventListener(VerifyMountPointEvent::class, ShareListener::class, 1000);
+		$context->registerEventListener(AttendeesRemovedEvent::class, ShareListener::class);
 		$context->registerEventListener(RoomDeletedEvent::class, ShareListener::class);
 
 		// Group and Circles listeners

lib/Migration/CleanupOldShares.php

@@ -0,0 +1,95 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Talk\Migration;
+
+use OCA\Talk\Model\Attendee;
+use OCA\Talk\Share\RoomShareProvider;
+use OCP\DB\QueryBuilder\IQueryBuilder;
+use OCP\IDBConnection;
+use OCP\Migration\IOutput;
+use OCP\Migration\IRepairStep;
+use OCP\Share\IShare;
+
+/**
+ * Remove old @see RoomShareProvider::SHARE_TYPE_USERROOM shares when the user
+ * is no longer part of the conversation
+ */
+class CleanupOldShares implements IRepairStep {
+
+	public const int SELECT_CHUNK_SIZE = 50_000;
+	public function __construct(
+		private readonly IDBConnection $connection,
+	) {
+	}
+
+	#[\Override]
+	public function getName(): string {
+		return 'Remove old user-room-shares';
+	}
+
+	#[\Override]
+	public function run(IOutput $output): void {
+		// Select SHARE_TYPE_USERROOM shares when the user is no longer an attendee in the room
+		$query = $this->connection->getQueryBuilder();
+		$query->select('su.id')
+			->from('share', 'su')
+			->leftJoin('su', 'share', 'sr', $query->expr()->andX(
+				$query->expr()->eq('su.parent', 'sr.id'),
+				$query->expr()->eq('sr.share_type', $query->createNamedParameter(IShare::TYPE_ROOM)),
+			))
+			->leftJoin('sr', 'talk_rooms', 'r', $query->expr()->eq('sr.share_with', 'r.token'))
+			->leftJoin('r', 'talk_attendees', 'a', $query->expr()->andX(
+				$query->expr()->eq('r.id', 'a.room_id'),
+				$query->expr()->eq('su.share_with', 'a.actor_id'),
+				$query->expr()->eq('a.actor_type', $query->createNamedParameter(Attendee::ACTOR_USERS)),
+			))
+			->where($query->expr()->eq('su.share_type', $query->createNamedParameter(RoomShareProvider::SHARE_TYPE_USERROOM)))
+			->andWhere($query->expr()->isNull('a.id'))
+			->setMaxResults(self::SELECT_CHUNK_SIZE);
+
+		// Deleting the SHARE_TYPE_USERROOM shares based on their ID
+		$delete = $this->connection->getQueryBuilder();
+		$delete->delete('share')
+			->where($delete->expr()->eq('share_type', $delete->createNamedParameter(RoomShareProvider::SHARE_TYPE_USERROOM)))
+			->andWhere($delete->expr()->in('id', $delete->createParameter('ids')));
+
+		$output->startProgress();
+		$numDeleted = 0;
+		do {
+			$deletedInChunk = $this->deleteSomeFormerShares($output, $query, $delete);
+			$numDeleted += $deletedInChunk;
+		} while ($deletedInChunk !== 0);
+		$output->finishProgress();
+
+		$output->info('Deleted ' . $numDeleted . ' stray shares');
+	}
+
+	protected function deleteSomeFormerShares(IOutput $output, IQueryBuilder $select, IQueryBuilder $delete): int {
+		$result = $select->executeQuery();
+		$ids = [];
+		while ($row = $result->fetchAssociative()) {
+			$ids[] = (int)$row['id'];
+		}
+		$result->closeCursor();
+
+		if (empty($ids)) {
+			return 0;
+		}
+
+		$count = 0;
+		$chunks = array_chunk($ids, IQueryBuilder::MAX_IN_PARAMETERS);
+		foreach ($chunks as $chunk) {
+			$delete->setParameter('ids', $chunk, IQueryBuilder::PARAM_INT_ARRAY);
+			$delete->executeStatement();
+			$output->advance(count($chunk));
+			$count += count($chunk);
+		}
+		return $count;
+	}
+}

lib/Share/Listener.php

@@ -10,9 +10,11 @@
 
 use OC\Files\Filesystem;
 use OCA\Talk\Config;
+use OCA\Talk\Events\AttendeesRemovedEvent;
 use OCA\Talk\Events\RoomDeletedEvent;
 use OCA\Talk\Exceptions\RoomNotFoundException;
 use OCA\Talk\Manager;
+use OCA\Talk\Model\Attendee;
 use OCA\Talk\Service\ConversationFolderService;
 use OCP\EventDispatcher\Event;
 use OCP\EventDispatcher\IEventListener;
@@ -38,6 +40,7 @@ public function handle(Event $event): void {
 			$event instanceof BeforeShareCreatedEvent => $this->overwriteShareTarget($event),
 			$event instanceof VerifyMountPointEvent => $this->overwriteMountPoint($event),
 			$event instanceof RoomDeletedEvent => $this->roomDeletedEvent($event),
+			$event instanceof AttendeesRemovedEvent => $this->roomAttendeesRemovedEvent($event),
 			default => null,
 		};
 	}
@@ -147,4 +150,16 @@ protected function overwriteMountPoint(VerifyMountPointEvent $event): void {
 	protected function roomDeletedEvent(RoomDeletedEvent $event): void {
 		$this->roomShareProvider->deleteInRoom($event->getRoom()->getToken());
 	}
+
+	protected function roomAttendeesRemovedEvent(AttendeesRemovedEvent $event): void {
+		$userIds = array_values(array_map(
+			static fn (Attendee $attendee): string => $attendee->getActorId(),
+			array_filter(
+				$event->getAttendees(),
+				static fn (Attendee $attendee): bool => $attendee->getActorType() === Attendee::ACTOR_USERS
+			)
+		));
+
+		$this->roomShareProvider->deleteReceivedSharesInRoom($event->getRoom()->getToken(), $userIds);
+	}
 }

lib/Share/RoomShareProvider.php

@@ -1234,18 +1234,14 @@ public function getChildren(IShare $parent): array {
 	}
 
 	/**
-	 * Delete all shares in a room, or only those from the given user.
-	 *
-	 * When a user is given all their shares are removed, both own shares and
-	 * received shares.
+	 * Delete all shares in a room
 	 *
 	 * Not part of IShareProvider API, but needed by the hooks in
-	 * OCA\Talk\AppInfo\Application
+	 * {@see Listener::roomDeletedEvent()}
 	 *
 	 * @param string $roomToken
-	 * @param string|null $user
 	 */
-	public function deleteInRoom(string $roomToken, ?string $user = null): void {
+	public function deleteInRoom(string $roomToken): void {
 		$this->cleanSharesByIdCache();
 
 		//First delete all custom room shares for the original shares to be removed
@@ -1255,10 +1251,6 @@ public function deleteInRoom(string $roomToken, ?string $user = null): void {
 			->where($qb->expr()->eq('share_type', $qb->createNamedParameter(IShare::TYPE_ROOM)))
 			->andWhere($qb->expr()->eq('share_with', $qb->createNamedParameter($roomToken)));
 
-		if ($user !== null) {
-			$qb->andWhere($qb->expr()->eq('uid_initiator', $qb->createNamedParameter($user)));
-		}
-
 		$cursor = $qb->executeQuery();
 		$ids = [];
 		while ($row = $cursor->fetchAssociative()) {
@@ -1285,36 +1277,49 @@ public function deleteInRoom(string $roomToken, ?string $user = null): void {
 		$delete->delete('share')
 			->where($delete->expr()->eq('share_type', $delete->createNamedParameter(IShare::TYPE_ROOM)))
 			->andWhere($delete->expr()->eq('share_with', $delete->createNamedParameter($roomToken)));
+		$delete->executeStatement();
+	}
 
-		if ($user !== null) {
-			$delete->andWhere($delete->expr()->eq('uid_initiator', $delete->createNamedParameter($user)));
-		}
+	/**
+	 * Delete all received shares for a user in a room
+	 *
+	 * Not part of IShareProvider API, but needed by the hooks in
+	 * {@see Listener::roomAttendeesRemovedEvent()}
+	 *
+	 * @param string $roomToken
+	 * @param list<string> $userIds
+	 */
+	public function deleteReceivedSharesInRoom(string $roomToken, array $userIds): void {
+		$this->cleanSharesByIdCache();
 
-		$delete->executeStatement();
+		$qb = $this->dbConnection->getQueryBuilder();
+		$qb->select('id')
+			->from('share')
+			->where($qb->expr()->eq('share_type', $qb->createNamedParameter(IShare::TYPE_ROOM)))
+			->andWhere($qb->expr()->eq('share_with', $qb->createNamedParameter($roomToken)));
 
-		// Finally delete all custom room shares leftovers for the given user
-		if ($user !== null) {
-			$query = $this->dbConnection->getQueryBuilder();
-			$query->select('id')
-				->from('share')
-				->where($query->expr()->eq('share_type', $query->createNamedParameter(IShare::TYPE_ROOM)))
-				->andWhere($query->expr()->eq('share_with', $query->createNamedParameter($roomToken)));
+		$cursor = $qb->executeQuery();
+		$ids = [];
+		while ($row = $cursor->fetchAssociative()) {
+			$ids[] = (int)$row['id'];
+		}
+		$cursor->closeCursor();
 
-			$cursor = $query->executeQuery();
-			$ids = [];
-			while ($row = $cursor->fetchAssociative()) {
-				$ids[] = (int)$row['id'];
-			}
-			$cursor->closeCursor();
+		if (!empty($ids)) {
+			$delete = $this->dbConnection->getQueryBuilder();
+			$delete->delete('share')
+				->where($delete->expr()->eq('share_type', $delete->createNamedParameter(self::SHARE_TYPE_USERROOM)))
+				->andWhere($delete->expr()->in('share_with', $delete->createParameter('userids')))
+				->andWhere($delete->expr()->in('parent', $delete->createParameter('ids')));
+
+			$chunkSize = min(100, IQueryBuilder::MAX_IN_PARAMETERS);
 
-			if (!empty($ids)) {
-				$chunkSize = min(100, IQueryBuilder::MAX_IN_PARAMETERS);
-				$chunks = array_chunk($ids, $chunkSize);
+			$userChunks = array_chunk($userIds, $chunkSize);
+			$chunks = array_chunk($ids, $chunkSize);
+			foreach ($userChunks as $userChunk) {
 				foreach ($chunks as $chunk) {
-					$delete->delete('share')
-						->where($delete->expr()->eq('share_type', $delete->createNamedParameter(self::SHARE_TYPE_USERROOM)))
-						->andWhere($delete->expr()->in('share_with', $delete->createNamedParameter($user)))
-						->andWhere($delete->expr()->in('parent', $delete->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY)));
+					$delete->setParameter('userids', $userChunk, IQueryBuilder::PARAM_STR_ARRAY)
+						->setParameter('ids', $chunk, IQueryBuilder::PARAM_INT_ARRAY);
 					$delete->executeStatement();
 				}
 			}