refactor: move imap flag encoding to helper

Signed-off-by: Daniel Kesselberg <[email protected]>
nextcloud · Sep 18, 2024 · aea8fe8 · aea8fe8
1 parent aa8e4e1
commit aea8fe8
Show file tree

Hide file tree

Showing 5 changed files with 107 additions and 8 deletions.
diff --git a/lib/Exception/ImapFlagEncodingException.php b/lib/Exception/ImapFlagEncodingException.php
@@ -0,0 +1,21 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Mail\Exception;
+
+use Exception;
+
+class ImapFlagEncodingException extends Exception {
+	public static function create($label): ImapFlagEncodingException {
+		return new self(
+			'Failed to convert the given label "' . $label . '" to UTF7-IMAP',
+			0,
+		);
+	}
+}
diff --git a/lib/IMAP/ImapFlag.php b/lib/IMAP/ImapFlag.php
@@ -0,0 +1,28 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Mail\IMAP;
+
+use OCA\Mail\Exception\ImapFlagEncodingException;
+
+class ImapFlag {
+	/**
+	 * @throws ImapFlagEncodingException
+	 */
+	public function create(string $label): string {
+		$flag = str_replace(' ', '_', $label);
+		$flag = mb_convert_encoding($flag, 'UTF7-IMAP', 'UTF-8');
+
+		if ($flag === false) {
+			throw ImapFlagEncodingException::create($label);
+		}
+
+		return '$' . strtolower(mb_strcut($flag, 0, 63));
+	}
+}
diff --git a/lib/Service/MailManager.php b/lib/Service/MailManager.php
@@ -29,12 +29,14 @@
 use OCA\Mail\Events\MessageDeletedEvent;
 use OCA\Mail\Events\MessageFlaggedEvent;
 use OCA\Mail\Exception\ClientException;
+use OCA\Mail\Exception\ImapFlagEncodingException;
 use OCA\Mail\Exception\ServiceException;
 use OCA\Mail\Exception\SmimeDecryptException;
 use OCA\Mail\Exception\TrashMailboxNotSetException;
 use OCA\Mail\Folder;
 use OCA\Mail\IMAP\FolderMapper;
 use OCA\Mail\IMAP\IMAPClientFactory;
+use OCA\Mail\IMAP\ImapFlag;
 use OCA\Mail\IMAP\MailboxSync;
 use OCA\Mail\IMAP\MessageMapper as ImapMessageMapper;
 use OCA\Mail\Model\IMAPMessage;
@@ -102,7 +104,8 @@ public function __construct(IMAPClientFactory $imapClientFactory,
 		LoggerInterface $logger,
 		TagMapper $tagMapper,
 		MessageTagsMapper $messageTagsMapper,
-		ThreadMapper $threadMapper) {
+		ThreadMapper $threadMapper,
+		private ImapFlag $imapFlag) {
 		$this->imapClientFactory = $imapClientFactory;
 		$this->mailboxMapper = $mailboxMapper;
 		$this->mailboxSync = $mailboxSync;
@@ -795,12 +798,11 @@ public function isPermflagsEnabled(Horde_Imap_Client_Socket $client, Account $ac
 	}
 
 	public function createTag(string $displayName, string $color, string $userId): Tag {
-		$imapLabel = str_replace(' ', '_', $displayName);
-		$imapLabel = mb_convert_encoding($imapLabel, 'UTF7-IMAP', 'UTF-8');
-		if ($imapLabel === false) {
-			throw new ClientException('Error converting display name to UTF7-IMAP ', 0);
+		try {
+			$imapLabel = $this->imapFlag->create($displayName);
+		} catch (ImapFlagEncodingException $e) {
+			throw new ClientException('Error converting display name to UTF7-IMAP ', 0, $e);
 		}
-		$imapLabel = '$' . strtolower(mb_strcut($imapLabel, 0, 63));
 
 		try {
 			return $this->getTagByImapLabel($imapLabel, $userId);

diff --git a/tests/Unit/IMAP/ImapFlagTest.php b/tests/Unit/IMAP/ImapFlagTest.php
@@ -0,0 +1,46 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace Unit\IMAP;
+
+use ChristophWurst\Nextcloud\Testing\TestCase;
+use OCA\Mail\IMAP\ImapFlag;
+use PHPUnit\Framework\Attributes\DataProvider;
+
+class ImapFlagTest extends TestCase {
+
+	private ImapFlag $imapFlag;
+
+	protected function setUp(): void {
+		parent::setUp();
+
+		$this->imapFlag = new ImapFlag();
+	}
+
+	/**
+	 * @dataProvider dataCreate
+	 */
+	public function testCreate(string $label, string $expected): void {
+		$actual = $this->imapFlag->create($label);
+		$this->assertEquals($expected, $actual);
+	}
+
+	public function dataCreate(): array {
+		return [
+			'umlauts and lowercase' => [
+				'Test ÄÖÜ',
+				'$test_&amqa1gdc-'
+			],
+			'maximum 63 characters' => [
+				'1234567890123456789012345678901234567890123456789012345678901234',
+				'$123456789012345678901234567890123456789012345678901234567890123',
+			],
+		];
+	}
+}
diff --git a/tests/Unit/Service/MailManagerTest.php b/tests/Unit/Service/MailManagerTest.php
@@ -27,6 +27,7 @@
 use OCA\Mail\Folder;
 use OCA\Mail\IMAP\FolderMapper;
 use OCA\Mail\IMAP\IMAPClientFactory;
+use OCA\Mail\IMAP\ImapFlag;
 use OCA\Mail\IMAP\MailboxSync;
 use OCA\Mail\IMAP\MessageMapper as ImapMessageMapper;
 use OCA\Mail\Service\MailManager;
@@ -72,7 +73,7 @@ class MailManagerTest extends TestCase {
 	/** @var ThreadMapper|MockObject */
 	private $threadMapper;
 
-	
+
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -100,7 +101,8 @@ protected function setUp(): void {
 			$this->logger,
 			$this->tagMapper,
 			$this->messageTagsMapper,
-			$this->threadMapper
+			$this->threadMapper,
+			new ImapFlag(),
 		);
 	}