fix(deletion): Clean up after deletion

lib/Listener/UserDeletedListener.php

@@ -12,6 +12,8 @@
 use OCA\Talk\Manager;
 use OCA\Talk\Matrix\Service\AccountService;
 use OCA\Talk\Model\Attendee;
+use OCA\Talk\Service\AttachmentService;
+use OCA\Talk\Service\BanService;
 use OCA\Talk\Service\ConsentService;
 use OCA\Talk\Service\ConversationTagService;
 use OCA\Talk\Service\PhoneService;
@@ -37,6 +39,8 @@ public function __construct(
 		private readonly ReminderService $reminderService,
 		private readonly FederationManager $federationManager,
 		private readonly AccountService $matrixAccountService,
+		private readonly BanService $banService,
+		private readonly AttachmentService $attachmentService,
 	) {
 	}
 
@@ -58,5 +62,7 @@ public function handle(Event $event): void {
 		$this->reminderService->deleteAllRemindersForUser($user->getUID());
 		$this->federationManager->deleteInvitationsForUser($user->getUID());
 		$this->matrixAccountService->unlinkUser($user->getUID());
+		$this->banService->neutralizeDeletedUser(Attendee::ACTOR_USERS, $user->getUID());
+		$this->attachmentService->neutralizeDeletedUser(Attendee::ACTOR_USERS, $user->getUID());
 	}
 }

lib/Model/AttachmentMapper.php

@@ -10,6 +10,7 @@
 
 use OCP\AppFramework\Db\QBMapper;
 use OCP\AppFramework\Db\TTransactional;
+use OCP\Comments\ICommentsManager;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\IDBConnection;
 
@@ -75,4 +76,14 @@ public function deleteByRoomId(int $roomId): void {
 			$query->executeStatement();
 		}, $this->db);
 	}
+
+	public function neutralizeDeletedUser(string $actorType, string $actorId): void {
+		$update = $this->db->getQueryBuilder();
+		$update->update($this->getTableName())
+			->set('actor_type', $update->createNamedParameter(ICommentsManager::DELETED_USER))
+			->set('actor_id', $update->createNamedParameter(ICommentsManager::DELETED_USER))
+			->where($update->expr()->eq('actor_type', $update->createNamedParameter($actorType)))
+			->andWhere($update->expr()->eq('actor_id', $update->createNamedParameter($actorId)));
+		$update->executeStatement();
+	}
 }

lib/Model/BanMapper.php

@@ -10,6 +10,7 @@
 
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Db\QBMapper;
+use OCP\Comments\ICommentsManager;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\IDBConnection;
 
@@ -95,4 +96,21 @@ public function updateDisplayNameForActor(string $actorType, string $actorId, st
 			->andWhere($update->expr()->eq('banned_actor_id', $update->createNamedParameter($actorId)));
 		$update->executeStatement();
 	}
+
+	public function neutralizeDeletedUser(string $actorType, string $actorId): void {
+		$delete = $this->db->getQueryBuilder();
+		$delete->delete($this->getTableName())
+			->where($delete->expr()->eq('banned_actor_type', $delete->createNamedParameter($actorType)))
+			->andWhere($delete->expr()->eq('banned_actor_id', $delete->createNamedParameter($actorId)));
+		$delete->executeStatement();
+
+		$update = $this->db->getQueryBuilder();
+		$update->update($this->getTableName())
+			->set('moderator_displayname', $update->createNamedParameter(''))
+			->set('moderator_actor_type', $update->createNamedParameter(ICommentsManager::DELETED_USER))
+			->set('moderator_actor_id', $update->createNamedParameter(ICommentsManager::DELETED_USER))
+			->where($update->expr()->eq('moderator_actor_type', $update->createNamedParameter($actorType)))
+			->andWhere($update->expr()->eq('moderator_actor_id', $update->createNamedParameter($actorId)));
+		$update->executeStatement();
+	}
 }

lib/Service/AttachmentService.php

@@ -92,4 +92,8 @@ public function deleteAttachmentByMessageId(int $messageId): void {
 	public function deleteAttachmentsForRoom(Room $room): void {
 		$this->attachmentMapper->deleteByRoomId($room->getId());
 	}
+
+	public function neutralizeDeletedUser(string $actorType, string $actorId): void {
+		$this->attachmentMapper->neutralizeDeletedUser($actorType, $actorId);
+	}
 }

lib/Service/AvatarService.php

@@ -301,12 +301,24 @@ private function getAvatarPath(Room $room, bool $darkTheme = false): string {
 	}
 
 	public function deleteAvatar(Room $room): void {
+		if ($this->deleteAvatarFolder($room)) {
+			$this->roomService->setAvatar($room, '');
+		}
+	}
+
+	/**
+	 * Delete the stored avatar files without updating the room
+	 *
+	 * @return bool Whether an avatar folder existed
+	 */
+	public function deleteAvatarFolder(Room $room): bool {
 		try {
 			$folder = $this->appData->getFolder('room-avatar');
 			$avatarFolder = $folder->getFolder($room->getToken());
 			$avatarFolder->delete();
-			$this->roomService->setAvatar($room, '');
+			return true;
 		} catch (NotFoundException) {
+			return false;
 		}
 	}
 

lib/Service/BanService.php

@@ -294,4 +294,11 @@ public function findAndDeleteBanByIdForRoom(int $banId, int $roomId): void {
 	public function updateDisplayNameForActor(string $actorType, string $actorId, string $displayName): void {
 		$this->banMapper->updateDisplayNameForActor($actorType, $actorId, $displayName);
 	}
+
+	/**
+	 * Delete bans of the actor and anonymize bans the actor issued as moderator
+	 */
+	public function neutralizeDeletedUser(string $actorType, string $actorId): void {
+		$this->banMapper->neutralizeDeletedUser($actorType, $actorId);
+	}
 }

lib/Service/RoomService.php

@@ -1576,6 +1576,22 @@ public function deleteRoom(Room $room): void {
 			->where($delete->expr()->eq('room_id', $delete->createNamedParameter($room->getId(), IQueryBuilder::PARAM_INT)));
 		$delete->executeStatement();
 
+		// Delete bans
+		$delete = $this->db->getQueryBuilder();
+		$delete->delete('talk_bans')
+			->where($delete->expr()->eq('room_id', $delete->createNamedParameter($room->getId(), IQueryBuilder::PARAM_INT)));
+		$delete->executeStatement();
+
+		// Delete bot installations
+		$delete = $this->db->getQueryBuilder();
+		$delete->delete('talk_bots_conversation')
+			->where($delete->expr()->eq('token', $delete->createNamedParameter($room->getToken())));
+		$delete->executeStatement();
+
+		// AvatarService depends on RoomService, so can not use DI
+		$avatarService = Server::get(AvatarService::class);
+		$avatarService->deleteAvatarFolder($room);
+
 		// Clear share access cache
 		foreach ($userIds as $userId) {
 			$user = new LazyUser($userId, $this->userManager);

tests/integration/features/conversation-5/delete-user.feature

@@ -74,3 +74,23 @@ Feature: conversation-5/delete-user
       | roomName | public room |
     When user "participant2" is deleted
     Then user "participant1" joins room "public room" with 404 (v4)
+
+  Scenario: delete user who is banned or banned others
+    Given user "participant3" exists
+    Given user "participant4" exists
+    Given user "participant1" creates room "room" (v4)
+      | roomType | 3 |
+      | roomName | room |
+    And user "participant1" adds user "participant3" to room "room" with 200 (v4)
+    And user "participant1" promotes "participant3" in room "room" with 200 (v4)
+    And user "participant2" joins room "room" with 200 (v4)
+    And user "participant4" joins room "room" with 200 (v4)
+    And user "participant3" bans user "participant2" from room "room" with 200 (v1)
+      | internalNote | BannedP2 |
+    And user "participant3" bans user "participant4" from room "room" with 200 (v1)
+      | internalNote | BannedP4 |
+    When user "participant2" is deleted
+    And user "participant3" is deleted
+    Then user "participant1" sees the following bans in room "room" with 200 (v1)
+      | moderatorActorType | moderatorActorId | moderatorDisplayName | bannedActorType | bannedActorId | bannedDisplayName        | internalNote |
+      | deleted_users      | deleted_users    |                      | users           | participant4  | participant4-displayname | BannedP4     |

tests/php/Service/AvatarServiceTest.php

@@ -15,6 +15,8 @@
 use OCP\Config\IUserConfig;
 use OCP\Files\IAppData;
 use OCP\Files\IFilenameValidator;
+use OCP\Files\NotFoundException;
+use OCP\Files\SimpleFS\ISimpleFolder;
 use OCP\IAvatarManager;
 use OCP\IL10N;
 use OCP\IURLGenerator;
@@ -101,4 +103,33 @@ public function testGetAvatarVersion(string $avatar, string $expected): void {
 			$this->assertEquals($expected, $actual);
 		}
 	}
+
+	public function testDeleteAvatar(): void {
+		$room = $this->createMock(Room::class);
+		$room->method('getToken')->willReturn('abcdef');
+
+		$avatarFolder = $this->createMock(ISimpleFolder::class);
+		$avatarFolder->expects($this->once())->method('delete');
+		$folder = $this->createMock(ISimpleFolder::class);
+		$folder->method('getFolder')->with('abcdef')->willReturn($avatarFolder);
+		$this->appData->method('getFolder')->with('room-avatar')->willReturn($folder);
+
+		$this->roomService->expects($this->once())->method('setAvatar')->with($room, '');
+
+		$this->service->deleteAvatar($room);
+	}
+
+	public function testDeleteAvatarWithoutFolder(): void {
+		$room = $this->createMock(Room::class);
+		$room->method('getToken')->willReturn('abcdef');
+
+		$folder = $this->createMock(ISimpleFolder::class);
+		$folder->method('getFolder')->willThrowException(new NotFoundException());
+		$this->appData->method('getFolder')->willReturn($folder);
+
+		$this->roomService->expects($this->never())->method('setAvatar');
+
+		$this->service->deleteAvatar($room);
+		$this->assertFalse($this->service->deleteAvatarFolder($room));
+	}
 }

tests/php/Service/RoomServiceTest.php

@@ -19,10 +19,15 @@
 use OCA\Talk\Exceptions\RoomProperty\TypeException;
 use OCA\Talk\Manager;
 use OCA\Talk\Model\Attendee;
+use OCA\Talk\Model\Ban;
+use OCA\Talk\Model\BanMapper;
+use OCA\Talk\Model\BotConversation;
+use OCA\Talk\Model\BotConversationMapper;
 use OCA\Talk\Model\BreakoutRoom;
 use OCA\Talk\Participant;
 use OCA\Talk\Room;
 use OCA\Talk\RoomAttributes;
+use OCA\Talk\Service\AvatarService;
 use OCA\Talk\Service\EmojiService;
 use OCA\Talk\Service\ParticipantService;
 use OCA\Talk\Service\RecordingService;
@@ -618,4 +623,53 @@ public function testDeleteRoomLogsCriticalAction(): void {
 			'objectId' => '42',
 		], $auditEvents[0]->getParameters());
 	}
+
+	public function testDeleteRoomDeletesRoomData(): void {
+		$banMapper = new BanMapper(Server::get(IDBConnection::class));
+		$botConversationMapper = new BotConversationMapper(Server::get(IDBConnection::class));
+		$roomId = random_int(1_000_000, 9_000_000);
+		$otherRoomId = $roomId + 1;
+		foreach ([$roomId, $otherRoomId] as $banRoomId) {
+			$ban = new Ban();
+			$ban->setRoomId($banRoomId);
+			$ban->setModeratorActorType(Attendee::ACTOR_USERS);
+			$ban->setModeratorActorId('moderator');
+			$ban->setBannedActorType(Attendee::ACTOR_USERS);
+			$ban->setBannedActorId('banned');
+			$ban->setBannedTime(new \DateTime());
+			$banMapper->insert($ban);
+
+			$bot = new BotConversation();
+			$bot->setBotId(1);
+			$bot->setToken('t' . $banRoomId);
+			$botConversationMapper->insert($bot);
+		}
+
+		$room = $this->createMock(Room::class);
+		$room->method('getId')->willReturn($roomId);
+		$room->method('getToken')->willReturn('t' . $roomId);
+		$room->method('getBreakoutRoomMode')->willReturn(BreakoutRoom::MODE_NOT_CONFIGURED);
+		$room->method('isFederatedConversation')->willReturn(false);
+		$this->participantService->method('getParticipantUserIds')->willReturn([]);
+
+		$avatarService = $this->createMock(AvatarService::class);
+		$avatarService->expects($this->once())->method('deleteAvatarFolder')->with($room);
+		$this->overwriteService(AvatarService::class, $avatarService);
+
+		try {
+			$this->service->deleteRoom($room);
+		} finally {
+			$this->restoreService(AvatarService::class);
+		}
+
+		$this->assertSame([], $banMapper->findByRoomId($roomId));
+		$this->assertSame([], $botConversationMapper->findForToken('t' . $roomId));
+
+		$otherBans = $banMapper->findByRoomId($otherRoomId);
+		$this->assertCount(1, $otherBans);
+		$banMapper->delete($otherBans[0]);
+		$otherBots = $botConversationMapper->findForToken('t' . $otherRoomId);
+		$this->assertCount(1, $otherBots);
+		$botConversationMapper->delete($otherBots[0]);
+	}
 }