From 48a149bab4a86d7af86c5b082bec9da57b807c70 Mon Sep 17 00:00:00 2001
From: Richard Steinmetz <richard@steinmetz.cloud>
Date: Tue, 2 Jul 2024 16:39:36 +0200
Subject: [PATCH] fix: duplicate uid repair job failing on postgres

Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
---
 lib/Db/MessageMapper.php                   | 21 +++++----
 tests/Integration/Db/MessageMapperTest.php | 54 +++++++++++++++++-----
 2 files changed, 56 insertions(+), 19 deletions(-)

diff --git a/lib/Db/MessageMapper.php b/lib/Db/MessageMapper.php
index bd1eab0602..c9da207d16 100644
--- a/lib/Db/MessageMapper.php
+++ b/lib/Db/MessageMapper.php
@@ -1662,6 +1662,8 @@ public function findMessagesToUnSnooze(int $mailboxId, int $time): array {
 	 * 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();
@@ -1672,14 +1674,7 @@ public function deleteDuplicateUids(): void {
 				$qb->expr()->eq('t1.uid', 't2.uid', IQueryBuilder::PARAM_INT),
 				$qb->expr()->neq('t1.id', 't2.id', IQueryBuilder::PARAM_INT),
 			))
-			->groupBy('mailbox_id', 'uid')
 			->executeQuery();
-		$rows = $result->fetchAll();
-		$result->closeCursor();
-
-		if (empty($rows)) {
-			return;
-		}
 
 		$deleteQb = $this->db->getQueryBuilder();
 		$deleteQb->delete($this->getTableName())
@@ -1701,11 +1696,21 @@ public function deleteDuplicateUids(): void {
 				),
 			);
 
-		foreach ($rows as $row) {
+		$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/tests/Integration/Db/MessageMapperTest.php b/tests/Integration/Db/MessageMapperTest.php
index ae2db99448..19746a3803 100644
--- a/tests/Integration/Db/MessageMapperTest.php
+++ b/tests/Integration/Db/MessageMapperTest.php
@@ -53,6 +53,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);
@@ -177,17 +191,7 @@ 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);
@@ -195,4 +199,32 @@ public function testDeleteByUid(): void {
 		$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]));
+	}
 }