Merge pull request #3438 from nextcloud/techdept/folder-mailbox

Move some usage of the old Folder to the more versatile Mailbox

lib/Contracts/IMailManager.php

@@ -24,6 +24,7 @@ declare(strict_types=1);
 namespace OCA\Mail\Contracts;
 
 use OCA\Mail\Account;
+use OCA\Mail\Db\Mailbox;
 use OCA\Mail\Exception\ClientException;
 use OCA\Mail\Exception\ServiceException;
 use OCA\Mail\Folder;
@@ -36,11 +37,11 @@ interface IMailManager {
 	/**
 	 * @param Account $account
 	 *
-	 * @return Folder[]
+	 * @return Mailbox[]
 	 *
 	 * @throws ServiceException
 	 */
-	public function getFolders(Account $account): array;
+	public function getMailboxes(Account $account): array;
 
 	/**
 	 * @param Account $account

lib/Controller/FoldersController.php

@@ -90,12 +90,12 @@ class FoldersController extends Controller {
 	public function index(int $accountId): JSONResponse {
 		$account = $this->accountService->find($this->currentUserId, $accountId);
 
-		$folders = $this->mailManager->getFolders($account);
+		$mailboxes = $this->mailManager->getMailboxes($account);
 		return new JSONResponse([
 			'id' => $accountId,
 			'email' => $account->getEmail(),
-			'folders' => $folders,
-			'delimiter' => reset($folders)->getDelimiter(),
+			'folders' => $mailboxes,
+			'delimiter' => reset($mailboxes)->getDelimiter(),
 		]);
 	}
 

lib/Controller/PageController.php

@@ -116,8 +116,8 @@ class PageController extends Controller {
 			$json['aliases'] = $this->aliasesService->findAll($mailAccount->getId(),
 				$this->currentUserId);
 			try {
-				$folders = $this->mailManager->getFolders($mailAccount);
-				$json['folders'] = $folders;
+				$mailboxes = $this->mailManager->getMailboxes($mailAccount);
+				$json['folders'] = $mailboxes;
 			} catch (Exception $ex) {
 				$this->logger->logException($ex, [
 					'message' => 'Could not load account folders: ' . $ex->getMessage(),

lib/Db/Mailbox.php

@@ -25,8 +25,9 @@ declare(strict_types=1);
 
 namespace OCA\Mail\Db;
 
-use OCA\Mail\Folder;
+use JsonSerializable;
 use OCP\AppFramework\Db\Entity;
+use function base64_encode;
 use function in_array;
 use function json_decode;
 use function ltrim;
@@ -62,7 +63,7 @@ use function strtolower;
  * @method string getSpecialUse()
  * @method void setSpecialUse(string $specialUse)
  */
-class Mailbox extends Entity {
+class Mailbox extends Entity implements JsonSerializable {
 	protected $name;
 	protected $accountId;
 	protected $syncNewToken;
@@ -88,19 +89,6 @@ class Mailbox extends Entity {
 		$this->addType('selectable', 'boolean');
 	}
 
-	public function toFolder(): Folder {
-		$folder = new Folder(
-			$this->accountId,
-			new \Horde_Imap_Client_Mailbox($this->name),
-			json_decode($this->getAttributes() ?? '[]', true) ?? [],
-			$this->delimiter
-		);
-		foreach ($this->getSpecialUseParsed() as $use) {
-			$folder->addSpecialUse($use);
-		}
-		return $folder;
-	}
-
 	private function getSpecialUseParsed(): array {
 		return json_decode($this->getSpecialUse() ?? '[]', true) ?? [];
 	}
@@ -127,4 +115,18 @@ class Mailbox extends Entity {
 			|| $this->getSyncChangedLock() !== null
 			|| $this->getSyncVanishedLock() !== null;
 	}
+
+	public function jsonSerialize() {
+		$specialUse = $this->getSpecialUseParsed();
+		return [
+			'databaseId' => $this->getId(),
+			'id' => base64_encode($this->getName()),
+			'accountId' => $this->accountId,
+			'displayName' => $this->getName(),
+			'attributes' => json_decode($this->attributes ?? '[]', true) ?? [],
+			'delimiter' => $this->delimiter,
+			'specialUse' => $specialUse,
+			'specialRole' => $specialUse[0] ?? 0,
+		];
+	}
 }

lib/Folder.php

@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 /**
  * @author Christoph Wurst <christoph@winzerhof-wurst.at>
  *
@@ -22,9 +24,8 @@
 namespace OCA\Mail;
 
 use Horde_Imap_Client_Mailbox;
-use JsonSerializable;
 
-class Folder implements JsonSerializable {
+class Folder {
 
 	/** @var int */
 	private $accountId;
@@ -38,16 +39,12 @@ class Folder implements JsonSerializable {
 	/** @var string */
 	private $delimiter;
 
-	/** @var Folder[] */
-	private $folders;
-
 	/** @var array */
 	private $status;
 
 	/** @var string[] */
 	private $specialUse;
 
-	/** @var string */
 	/**
 	 * @param Account $account
 	 * @param Horde_Imap_Client_Mailbox $mailbox
@@ -59,7 +56,6 @@ class Folder implements JsonSerializable {
 		$this->mailbox = $mailbox;
 		$this->attributes = $attributes;
 		$this->delimiter = $delimiter;
-		$this->folders = [];
 		$this->status = [];
 		$this->specialUse = [];
 	}
@@ -78,22 +74,10 @@ class Folder implements JsonSerializable {
 		return $this->delimiter;
 	}
 
-	/**
-	 * @return array
-	 */
-	public function getAttributes() {
+	public function getAttributes(): array {
 		return $this->attributes;
 	}
 
-	/**
-	 * @param Folder $folder
-	 *
-	 * @return void
-	 */
-	public function addFolder(Folder $folder): void {
-		$this->folders[$folder->getMailbox()] = $folder;
-	}
-
 	/**
 	 * @param array $status
 	 *
@@ -118,46 +102,4 @@ class Folder implements JsonSerializable {
 	public function getSpecialUse() {
 		return $this->specialUse;
 	}
-
-	/**
-	 * @return Folder[]
-	 */
-	public function getFolders() {
-		return $this->folders;
-	}
-
-	/**
-	 * @return boolean
-	 */
-	public function isSearchable() {
-		return !in_array('\noselect', $this->getAttributes());
-	}
-
-	/**
-	 * @return array
-	 */
-	public function jsonSerialize() {
-		$folders = [];
-		foreach ($this->folders as $folder) {
-			$folders[$folder->getMailbox()] = $folder->jsonSerialize();
-		}
-		return [
-			'id' => base64_encode($this->getMailbox()),
-			'accountId' => $this->accountId,
-			'displayName' => $this->getMailbox(),
-			'unseen' => isset($this->status['unseen']) ? $this->status['unseen'] : 0,
-			'total' => isset($this->status['messages']) ? (int) $this->status['messages'] : 0,
-			'isEmpty' => isset($this->status['messages']) ? 0 >= (int) $this->status['messages'] : true,
-			'noSelect' => !$this->isSelectable(),
-			'attributes' => $this->attributes,
-			'delimiter' => $this->delimiter,
-			'folders' => array_values($folders),
-			'specialUse' => $this->specialUse,
-			'specialRole' => empty($this->specialUse) ? null : $this->specialUse[0],
-		];
-	}
-
-	public function isSelectable(): bool {
-		return !in_array('\noselect', $this->attributes);
-	}
 }

lib/IMAP/FolderMapper.php

@@ -109,7 +109,7 @@ class FolderMapper {
 		$mailboxes = array_map(function (Folder $folder) {
 			return $folder->getMailbox();
 		}, array_filter($folders, function (Folder $folder) {
-			return $folder->isSearchable();
+			return !in_array('\noselect', $folder->getAttributes());
 		}));
 
 		$status = $client->status($mailboxes);

lib/IMAP/MailboxSync.php

@@ -27,6 +27,7 @@ namespace OCA\Mail\IMAP;
 
 use Horde_Imap_Client_Exception;
 use OCA\Mail\Exception\ServiceException;
+use function in_array;
 use function json_encode;
 use OCA\Mail\Account;
 use OCA\Mail\Db\MailAccountMapper;
@@ -130,7 +131,7 @@ class MailboxSync {
 		$mailbox->setDelimiter($folder->getDelimiter());
 		$mailbox->setMessages(0); // TODO
 		$mailbox->setUnseen(0); // TODO
-		$mailbox->setSelectable($folder->isSelectable());
+		$mailbox->setSelectable(!in_array('\noselect', $folder->getAttributes()));
 		$mailbox->setSpecialUse(json_encode($folder->getSpecialUse()));
 		$this->mailboxMapper->update($mailbox);
 	}
@@ -143,7 +144,7 @@ class MailboxSync {
 		$mailbox->setDelimiter($folder->getDelimiter());
 		$mailbox->setMessages(0); // TODO
 		$mailbox->setUnseen(0); // TODO
-		$mailbox->setSelectable($folder->isSelectable());
+		$mailbox->setSelectable(!in_array('\noselect', $folder->getAttributes()));
 		$mailbox->setSpecialUse(json_encode($folder->getSpecialUse()));
 		$this->mailboxMapper->insert($mailbox);
 	}

lib/Service/MailManager.php

@@ -103,18 +103,13 @@ class MailManager implements IMailManager {
 	/**
 	 * @param Account $account
 	 *
-	 * @return Folder[]
+	 * @return Mailbox[]
 	 * @throws ServiceException
 	 */
-	public function getFolders(Account $account): array {
+	public function getMailboxes(Account $account): array {
 		$this->mailboxSync->sync($account);
 
-		return array_map(
-			function (Mailbox $mb) {
-				return $mb->toFolder();
-			},
-			$this->mailboxMapper->findAll($account)
-		);
+		return $this->mailboxMapper->findAll($account);
 	}
 
 	/**

tests/FolderTest.php

@@ -66,18 +66,6 @@ class FolderTest extends TestCase {
 		$this->assertSame(['\noselect'], $this->folder->getAttributes());
 	}
 
-	public function testAddFolder() {
-		$this->mockFolder(['\noselect']);
-		$subFolder = $this->createMock(Folder::class);
-		$subFolder->expects($this->once())
-			->method('getMailbox')
-			->willReturn('INBOX/FLAGED');
-
-		$this->folder->addFolder($subFolder);
-
-		$this->assertCount(1, $this->folder->getFolders());
-	}
-
 	public function testSetStatus() {
 		$this->mockFolder();
 
@@ -96,57 +84,4 @@ class FolderTest extends TestCase {
 		$this->assertCount(1, $this->folder->getSpecialUse());
 		$this->assertSame('flagged', $this->folder->getSpecialUse()[0]);
 	}
-
-	public function testIsSearchable() {
-		$this->mockFolder([]);
-
-		$this->assertTrue($this->folder->isSearchable());
-	}
-
-	public function testIsNotSearchable() {
-		$this->mockFolder(['\noselect']);
-
-		$this->assertFalse($this->folder->isSearchable());
-	}
-
-	public function testJsonSerialize() {
-		$this->mockFolder();
-		$subFolder = $this->createMock(Folder::class);
-
-		$subFolder->expects($this->exactly(2))
-			->method('getMailbox')
-			->willReturn('Archive');
-		$subFolder->expects($this->once())
-			->method('jsonSerialize')
-			->willReturn(['subdir data']);
-		$this->mailbox->expects($this->exactly(2))
-			->method('__get')
-			->with($this->equalTo('utf8'))
-			->willReturn('Sent');
-
-		$this->folder->addSpecialUse('sent');
-		$this->folder->setStatus([
-			'unseen' => 13,
-			'messages' => 333,
-		]);
-		$this->folder->addFolder($subFolder);
-
-		$expected = [
-			'id' => base64_encode('Sent'),
-			'accountId' => 15,
-			'displayName' => 'Sent',
-			'specialRole' => null,
-			'unseen' => 13,
-			'total' => 333,
-			'isEmpty' => false,
-			'noSelect' => false,
-			'attributes' => [],
-			'delimiter' => '.',
-			'folders' => [['subdir data']],
-			'specialRole' => 'sent',
-			'specialUse' => ['sent'],
-			'syncToken' => null,
-		];
-		$this->assertEquals($expected, $this->folder->jsonSerialize());
-	}
 }

tests/Unit/Controller/FoldersControllerTest.php

@@ -86,7 +86,7 @@ class FoldersControllerTest extends TestCase {
 			->with($this->equalTo($this->userId), $this->equalTo($accountId))
 			->willReturn($account);
 		$this->mailManager->expects($this->once())
-			->method('getFolders')
+			->method('getMailboxes')
 			->with($this->equalTo($account))
 			->willReturn([
 				$folder

tests/Unit/Controller/PageControllerTest.php

@@ -27,7 +27,7 @@ use ChristophWurst\Nextcloud\Testing\TestCase;
 use OCA\Mail\Account;
 use OCA\Mail\Contracts\IUserPreferences;
 use OCA\Mail\Controller\PageController;
-use OCA\Mail\Folder;
+use OCA\Mail\Db\Mailbox;
 use OCA\Mail\Service\AccountService;
 use OCA\Mail\Service\AliasesService;
 use OCA\Mail\Service\MailManager;
@@ -119,7 +119,7 @@ class PageControllerTest extends TestCase {
 	public function testIndex() {
 		$account1 = $this->createMock(Account::class);
 		$account2 = $this->createMock(Account::class);
-		$folder = $this->createMock(Folder::class);
+		$mailbox = $this->createMock(Mailbox::class);
 		$this->preferences->expects($this->exactly(2))
 			->method('getPreference')
 			->willReturnMap([
@@ -134,11 +134,11 @@ class PageControllerTest extends TestCase {
 				$account2,
 			]));
 		$this->mailManager->expects($this->at(0))
-			->method('getFolders')
+			->method('getMailboxes')
 			->with($account1)
-			->willReturn([$folder]);
+			->willReturn([$mailbox]);
 		$this->mailManager->expects($this->at(1))
-			->method('getFolders')
+			->method('getMailboxes')
 			->with($account2)
 			->willReturn([]);
 		$account1->expects($this->once())
@@ -146,7 +146,7 @@ class PageControllerTest extends TestCase {
 			->will($this->returnValue([
 				'accountId' => 1,
 			]));
-		$folder->expects($this->once())
+		$mailbox->expects($this->once())
 			->method('jsonSerialize')
 			->willReturn(['id' => 'inbox']);
 		$account1->expects($this->once())

tests/Unit/IMAP/FolderMapperTest.php

@@ -135,8 +135,8 @@ class FolderMapperTest extends TestCase {
 			->method('getMailbox')
 			->willReturn('folder1');
 		$folders[0]->expects($this->once())
-			->method('isSearchable')
-			->willReturn(true);
+			->method('getAttributes')
+			->willReturn([]);
 		$client->expects($this->once())
 			->method('status')
 			->with($this->equalTo(['folder1']))
@@ -160,8 +160,8 @@ class FolderMapperTest extends TestCase {
 			->method('getMailbox')
 			->willReturn('folder1');
 		$folders[0]->expects($this->once())
-			->method('isSearchable')
-			->willReturn(true);
+			->method('getAttributes')
+			->willReturn([]);
 		$client->expects($this->once())
 			->method('status')
 			->with($this->equalTo(['folder1']))
@@ -183,8 +183,8 @@ class FolderMapperTest extends TestCase {
 			->method('getMailbox')
 			->willReturn('folder1');
 		$folders[0]->expects($this->once())
-			->method('isSearchable')
-			->willReturn(false);
+			->method('getAttributes')
+			->willReturn(['\\noselect']);
 		$client->expects($this->once())
 			->method('status')
 			->with($this->equalTo([]))

tests/Unit/Service/MailManagerTest.php

@@ -97,12 +97,6 @@ class MailManagerTest extends TestCase {
 			$this->createMock(Mailbox::class),
 			$this->createMock(Mailbox::class),
 		];
-		$folders = [
-			$this->createMock(Folder::class),
-			$this->createMock(Folder::class),
-		];
-		$mailboxes[0]->method('toFolder')->willReturn($folders[0]);
-		$mailboxes[1]->method('toFolder')->willReturn($folders[1]);
 		$this->mailboxSync->expects($this->once())
 			->method('sync')
 			->with($this->equalTo($account));
@@ -111,9 +105,9 @@ class MailManagerTest extends TestCase {
 			->with($this->equalTo($account))
 			->willReturn($mailboxes);
 
-		$result = $this->manager->getFolders($account);
+		$result = $this->manager->getMailboxes($account);
 
-		$this->assertSame($folders, $result);
+		$this->assertSame($mailboxes, $result);
 	}
 
 	public function testCreateFolder() {