Merge pull request #9789 from nextcloud/backport/9765/stable3.7

[stable3.7] fix: add repair job to deleted duplicated cached messages
nextcloud · Jul 8, 2024 · df72a81 · df72a81
2 parents 9182096 + 48f1b33
commit df72a81
Show file tree

Hide file tree

Showing 4 changed files with 129 additions and 11 deletions.
diff --git a/appinfo/info.xml b/appinfo/info.xml
@@ -66,6 +66,7 @@ Learn more about the Nextcloud Ethical AI Rating [in our blog](https://nextcloud
 			<step>OCA\Mail\Migration\MakeItineraryExtractorExecutable</step>
 			<step>OCA\Mail\Migration\ProvisionAccounts</step>
 			<step>OCA\Mail\Migration\RepairMailTheads</step>
+			<step>OCA\Mail\Migration\DeleteDuplicateUids</step>
 		</post-migration>
 	</repair-steps>
 	<commands>

diff --git a/lib/Db/MessageMapper.php b/lib/Db/MessageMapper.php
@@ -1641,4 +1641,60 @@ public function findMessagesToUnSnooze(int $mailboxId, int $time): array {
 
 		return $this->findEntities($select);
 	}
+
+	/**
+	 * Delete all duplicated cached messages.
+	 * Some messages (with the same mailbox_id and uid) where inserted twice and this method cleans
+	 * up the duplicated rows.
+	 *
+	 * @throws \OCP\DB\Exception
+	 */
+	public function deleteDuplicateUids(): void {
+		$qb = $this->db->getQueryBuilder();
+		$result = $qb->select('t1.id', 't1.mailbox_id', 't1.uid')
+			->from($this->getTableName(), 't1')
+			->innerJoin('t1', $this->getTableName(), 't2', $qb->expr()->andX(
+				$qb->expr()->eq('t1.mailbox_id', 't2.mailbox_id', IQueryBuilder::PARAM_INT),
+				$qb->expr()->eq('t1.uid', 't2.uid', IQueryBuilder::PARAM_INT),
+				$qb->expr()->neq('t1.id', 't2.id', IQueryBuilder::PARAM_INT),
+			))
+			->executeQuery();
+
+		$deleteQb = $this->db->getQueryBuilder();
+		$deleteQb->delete($this->getTableName())
+			->where(
+				$deleteQb->expr()->neq(
+					'id',
+					$deleteQb->createParameter('id'),
+					IQueryBuilder::PARAM_INT,
+				),
+				$deleteQb->expr()->eq(
+					'mailbox_id',
+					$deleteQb->createParameter('mailbox_id'),
+					IQueryBuilder::PARAM_INT,
+				),
+				$deleteQb->expr()->eq(
+					'uid',
+					$deleteQb->createParameter('uid'),
+					IQueryBuilder::PARAM_INT,
+				),
+			);
+
+		$handledMailboxIdUidPairs = [];
+		while ($row = $result->fetch()) {
+			$pair = $row['mailbox_id'] . ':' . $row['uid'];
+			if (isset($handledMailboxIdUidPairs[$pair])) {
+				continue;
+			}
+
+			$deleteQb->setParameter('id', $row['id'], IQueryBuilder::PARAM_INT);
+			$deleteQb->setParameter('mailbox_id', $row['mailbox_id'], IQueryBuilder::PARAM_INT);
+			$deleteQb->setParameter('uid', $row['uid'], IQueryBuilder::PARAM_INT);
+			$deleteQb->executeStatement();
+
+			$handledMailboxIdUidPairs[$pair] = true;
+		}
+
+		$result->closeCursor();
+	}
 }
diff --git a/lib/Migration/DeleteDuplicateUids.php b/lib/Migration/DeleteDuplicateUids.php
@@ -0,0 +1,29 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Mail\Migration;
+
+use OCA\Mail\Db\MessageMapper;
+use OCP\Migration\IOutput;
+use OCP\Migration\IRepairStep;
+
+class DeleteDuplicateUids implements IRepairStep {
+	public function __construct(
+		private MessageMapper $messageMapper,
+	) {
+	}
+
+	public function getName(): string {
+		return 'Delete duplicated cached messages';
+	}
+
+	public function run(IOutput $output): void {
+		$this->messageMapper->deleteDuplicateUids();
+	}
+}
diff --git a/tests/Integration/Db/MessageMapperTest.php b/tests/Integration/Db/MessageMapperTest.php
@@ -69,6 +69,20 @@ protected function setUp(): void {
 		$delete->executeStatement();
 	}
 
+	private function insertMessage(int $uid, int $mailbox_id): void {
+		$qb = $this->db->getQueryBuilder();
+		$insert = $qb->insert($this->mapper->getTableName())
+			->values([
+				'uid' => $qb->createNamedParameter($uid, IQueryBuilder::PARAM_INT),
+				'message_id' => $qb->createNamedParameter('<abc' . $uid . $mailbox_id . '@123.com>'),
+				'mailbox_id' => $qb->createNamedParameter($mailbox_id, IQueryBuilder::PARAM_INT),
+				'subject' => $qb->createNamedParameter('TEST'),
+				'sent_at' => $qb->createNamedParameter(time(), IQueryBuilder::PARAM_INT),
+				'in_reply_to' => $qb->createNamedParameter('<>')
+			]);
+		$insert->executeStatement();
+	}
+
 	public function testResetInReplyTo() : void {
 		$account = $this->createMock(Account::class);
 		$account->method('getId')->willReturn(13);
@@ -193,22 +207,40 @@ public function testDeleteByUid(): void {
 		$mailbox = new Mailbox();
 		$mailbox->setId(1);
 		array_map(function ($i) {
-			$qb = $this->db->getQueryBuilder();
-			$insert = $qb->insert($this->mapper->getTableName())
-				->values([
-					'uid' => $qb->createNamedParameter($i, IQueryBuilder::PARAM_INT),
-					'message_id' => $qb->createNamedParameter('<abc' . $i . '@123.com>'),
-					'mailbox_id' => $qb->createNamedParameter(1, IQueryBuilder::PARAM_INT),
-					'subject' => $qb->createNamedParameter('TEST'),
-					'sent_at' => $qb->createNamedParameter(time(), IQueryBuilder::PARAM_INT),
-					'in_reply_to' => $qb->createNamedParameter('<>')
-				]);
-			$insert->executeStatement();
+			$this->insertMessage($i, 1);
 		}, range(1, 10));
 
 		$this->mapper->deleteByUid($mailbox, 1, 5);
 
 		$messages = $this->mapper->findByUids($mailbox, range(1, 10));
 		self::assertCount(8, $messages);
 	}
+
+	public function testDeleteDuplicateUids(): void {
+		$mailbox1 = new Mailbox();
+		$mailbox1->setId(1);
+		$mailbox2 = new Mailbox();
+		$mailbox2->setId(2);
+		$mailbox3 = new Mailbox();
+		$mailbox3->setId(3);
+		$this->insertMessage(100, 1);
+		$this->insertMessage(101, 1);
+		$this->insertMessage(101, 1);
+		$this->insertMessage(102, 1);
+		$this->insertMessage(102, 1);
+		$this->insertMessage(102, 1);
+		$this->insertMessage(103, 2);
+		$this->insertMessage(104, 2);
+		$this->insertMessage(104, 2);
+		$this->insertMessage(105, 3);
+
+		$this->mapper->deleteDuplicateUids();
+
+		self::assertCount(1, $this->mapper->findByUids($mailbox1, [100]));
+		self::assertCount(1, $this->mapper->findByUids($mailbox1, [101]));
+		self::assertCount(1, $this->mapper->findByUids($mailbox1, [102]));
+		self::assertCount(1, $this->mapper->findByUids($mailbox2, [103]));
+		self::assertCount(1, $this->mapper->findByUids($mailbox2, [104]));
+		self::assertCount(1, $this->mapper->findByUids($mailbox3, [105]));
+	}
 }