Skip to content

Commit fdcd70c

Browse files
ChristophWurstChristophWurst
authored
Merge pull request #8456 from nextcloud/backport/8449/stable2.2
[stable2.2] fix(imap): Only fetch mailbox STATUS once
2 parents 46b72ad + edcbdb6 commit fdcd70c

File tree

7 files changed

+37
-123
lines changed

7 files changed

+37
-123
lines changed

‎lib/Folder.php‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,16 @@ class Folder {
4444
/** @var string[] */
4545
private $specialUse;
4646

47-
public function __construct(int $accountId, Horde_Imap_Client_Mailbox $mailbox, array $attributes, ?string $delimiter) {
47+
public function __construct(int $accountId,
48+
Horde_Imap_Client_Mailbox $mailbox,
49+
array $attributes,
50+
?string $delimiter,
51+
array $status) {
4852
$this->accountId = $accountId;
4953
$this->mailbox = $mailbox;
5054
$this->attributes = $attributes;
5155
$this->delimiter = $delimiter;
52-
$this->status = [];
56+
$this->status = $status;
5357
$this->specialUse = [];
5458
}
5559

‎lib/IMAP/FolderMapper.php‎

Lines changed: 12 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -59,31 +59,21 @@ public function getFolders(Account $account, Horde_Imap_Client_Socket $client,
5959
'delimiter' => true,
6060
'attributes' => true,
6161
'special_use' => true,
62+
'status' => Horde_Imap_Client::STATUS_ALL,
6263
]);
6364

64-
return array_filter(array_map(function (array $mailbox) use ($account, $client) {
65+
return array_filter(array_map(static function (array $mailbox) use ($account) {
6566
if (in_array($mailbox['mailbox']->utf8, self::DOVECOT_SIEVE_FOLDERS, true)) {
6667
// This is a special folder that must not be shown
6768
return null;
6869
}
6970

70-
try {
71-
$client->status($mailbox["mailbox"]);
72-
} catch (Horde_Imap_Client_Exception $e) {
73-
// ignore folders which cause errors on access
74-
// (i.e. server-side system I/O errors)
75-
if (in_array($e->getCode(), [
76-
Horde_Imap_Client_Exception::UNSPECIFIED,
77-
], true)) {
78-
return null;
79-
}
80-
}
81-
8271
return new Folder(
8372
$account->getId(),
8473
$mailbox['mailbox'],
8574
$mailbox['attributes'],
86-
$mailbox['delimiter']
75+
$mailbox['delimiter'],
76+
$mailbox['status'],
8777
);
8878
}, $mailboxes));
8979
}
@@ -97,39 +87,21 @@ public function createFolder(Horde_Imap_Client_Socket $client,
9787
'delimiter' => true,
9888
'attributes' => true,
9989
'special_use' => true,
90+
'status' => Horde_Imap_Client::STATUS_ALL,
10091
]);
10192
$mb = reset($list);
10293

10394
if ($mb === null) {
10495
throw new ServiceException("Created mailbox does not exist");
10596
}
10697

107-
return new Folder($account->getId(), $mb['mailbox'], $mb['attributes'], $mb['delimiter']);
108-
}
109-
110-
/**
111-
* @param Folder[] $folders
112-
* @param Horde_Imap_Client_Socket $client
113-
*
114-
* @throws Horde_Imap_Client_Exception
115-
*
116-
* @return void
117-
*/
118-
public function getFoldersStatus(array $folders,
119-
Horde_Imap_Client_Socket $client): void {
120-
$mailboxes = array_map(function (Folder $folder) {
121-
return $folder->getMailbox();
122-
}, array_filter($folders, function (Folder $folder) {
123-
return !in_array('\noselect', $folder->getAttributes());
124-
}));
125-
126-
$status = $client->status($mailboxes);
127-
128-
foreach ($folders as $folder) {
129-
if (isset($status[$folder->getMailbox()])) {
130-
$folder->setStatus($status[$folder->getMailbox()]);
131-
}
132-
}
98+
return new Folder(
99+
$account->getId(),
100+
$mb['mailbox'],
101+
$mb['attributes'],
102+
$mb['delimiter'],
103+
$mb['status'],
104+
);
133105
}
134106

135107
/**

‎lib/IMAP/MailboxSync.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,6 @@ public function sync(Account $account,
102102

103103
try {
104104
$folders = $this->folderMapper->getFolders($account, $client);
105-
$this->folderMapper->getFoldersStatus($folders, $client);
106105
} catch (Horde_Imap_Client_Exception $e) {
107106
throw new ServiceException(
108107
sprintf("IMAP error synchronizing account %d: %s", $account->getId(), $e->getMessage()),

‎lib/Service/MailManager.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,6 @@ public function createMailbox(Account $account, string $name): Mailbox {
153153
$client = $this->imapClientFactory->getClient($account);
154154
try {
155155
$folder = $this->folderMapper->createFolder($client, $account, $name);
156-
$this->folderMapper->getFoldersStatus([$folder], $client);
157156
} catch (Horde_Imap_Client_Exception $e) {
158157
throw new ServiceException(
159158
"Could not get mailbox status: " .

‎tests/FolderTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ private function mockFolder(array $attributes = [], $delimiter = '.') {
4040
$this->accountId = 15;
4141
$this->mailbox = $this->createMock(Horde_Imap_Client_Mailbox::class);
4242

43-
$this->folder = new Folder($this->accountId, $this->mailbox, $attributes, $delimiter);
43+
$this->folder = new Folder($this->accountId, $this->mailbox, $attributes, $delimiter, []);
4444
}
4545

4646
public function testGetMailbox() {

‎tests/Unit/IMAP/FolderMapperTest.php‎

Lines changed: 18 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ protected function setUp(): void {
4242
$this->mapper = new FolderMapper();
4343
}
4444

45-
public function testGetFoldersEmtpyAccount() {
45+
public function testGetFoldersEmtpyAccount(): void {
4646
$account = $this->createMock(Account::class);
4747
$client = $this->createMock(Horde_Imap_Client_Socket::class);
4848
$client->expects($this->once())
@@ -52,6 +52,7 @@ public function testGetFoldersEmtpyAccount() {
5252
'delimiter' => true,
5353
'attributes' => true,
5454
'special_use' => true,
55+
'status' => Horde_Imap_Client::STATUS_ALL,
5556
]))
5657
->willReturn([]);
5758

@@ -60,7 +61,7 @@ public function testGetFoldersEmtpyAccount() {
6061
$this->assertEquals([], $folders);
6162
}
6263

63-
public function testGetFolders() {
64+
public function testGetFolders(): void {
6465
$account = $this->createMock(Account::class);
6566
$account->method('getId')->willReturn(27);
6667
$client = $this->createMock(Horde_Imap_Client_Socket::class);
@@ -71,32 +72,39 @@ public function testGetFolders() {
7172
'delimiter' => true,
7273
'attributes' => true,
7374
'special_use' => true,
75+
'status' => Horde_Imap_Client::STATUS_ALL,
7476
]))
7577
->willReturn([
7678
[
7779
'mailbox' => new Horde_Imap_Client_Mailbox('INBOX'),
7880
'attributes' => [],
7981
'delimiter' => '.',
82+
'status' => [
83+
'unseen' => 0,
84+
],
8085
],
8186
[
8287
'mailbox' => new Horde_Imap_Client_Mailbox('Sent'),
8388
'attributes' => [
8489
'\sent',
8590
],
8691
'delimiter' => '.',
92+
'status' => [
93+
'unseen' => 1,
94+
],
8795
],
8896
]);
8997
$expected = [
90-
new Folder(27, new Horde_Imap_Client_Mailbox('INBOX'), [], '.'),
91-
new Folder(27, new Horde_Imap_Client_Mailbox('Sent'), ['\sent'], '.'),
98+
new Folder(27, new Horde_Imap_Client_Mailbox('INBOX'), [], '.', ['unseen' => 0]),
99+
new Folder(27, new Horde_Imap_Client_Mailbox('Sent'), ['\sent'], '.', ['unseen' => 1]),
92100
];
93101

94102
$folders = $this->mapper->getFolders($account, $client);
95103

96104
$this->assertEquals($expected, $folders);
97105
}
98106

99-
public function testCreateFolder() {
107+
public function testCreateFolder(): void {
100108
$account = $this->createMock(Account::class);
101109
$account->method('getId')->willReturn(42);
102110
$client = $this->createMock(Horde_Imap_Client_Socket::class);
@@ -110,90 +118,25 @@ public function testCreateFolder() {
110118
'delimiter' => true,
111119
'attributes' => true,
112120
'special_use' => true,
121+
'status' => Horde_Imap_Client::STATUS_ALL,
113122
]))
114123
->willReturn([
115124
[
116125
'mailbox' => new Horde_Imap_Client_Mailbox('new'),
117126
'attributes' => [],
118127
'delimiter' => '.',
128+
'status' => [
129+
'unseen' => 0,
130+
],
119131
],
120132
]);
121133

122134
$created = $this->mapper->createFolder($client, $account, 'new');
123135

124-
$expected = new Folder(42, new Horde_Imap_Client_Mailbox('new'), [], '.');
136+
$expected = new Folder(42, new Horde_Imap_Client_Mailbox('new'), [], '.', ['unseen' => 0]);
125137
$this->assertEquals($expected, $created);
126138
}
127139

128-
public function testGetFoldersStatus() {
129-
$folders = [
130-
$this->createMock(Folder::class),
131-
];
132-
$client = $this->createMock(Horde_Imap_Client_Socket::class);
133-
$folders[0]->expects($this->any())
134-
->method('getMailbox')
135-
->willReturn('folder1');
136-
$folders[0]->expects($this->once())
137-
->method('getAttributes')
138-
->willReturn([]);
139-
$client->expects($this->once())
140-
->method('status')
141-
->with($this->equalTo(['folder1']))
142-
->willReturn([
143-
'folder1' => [
144-
'total' => 123
145-
],
146-
]);
147-
$folders[0]->expects($this->once())
148-
->method('setStatus');
149-
150-
$this->mapper->getFoldersStatus($folders, $client);
151-
}
152-
153-
public function testGetFoldersStatusNoStatusReported() {
154-
$folders = [
155-
$this->createMock(Folder::class),
156-
];
157-
$client = $this->createMock(Horde_Imap_Client_Socket::class);
158-
$folders[0]->expects($this->any())
159-
->method('getMailbox')
160-
->willReturn('folder1');
161-
$folders[0]->expects($this->once())
162-
->method('getAttributes')
163-
->willReturn([]);
164-
$client->expects($this->once())
165-
->method('status')
166-
->with($this->equalTo(['folder1']))
167-
->willReturn([
168-
// Nothing reported for this folder
169-
]);
170-
$folders[0]->expects($this->never())
171-
->method('setStatus');
172-
173-
$this->mapper->getFoldersStatus($folders, $client);
174-
}
175-
176-
public function testGetFoldersStatusNotSearchable() {
177-
$folders = [
178-
$this->createMock(Folder::class),
179-
];
180-
$client = $this->createMock(Horde_Imap_Client_Socket::class);
181-
$folders[0]->expects($this->any())
182-
->method('getMailbox')
183-
->willReturn('folder1');
184-
$folders[0]->expects($this->once())
185-
->method('getAttributes')
186-
->willReturn(['\\noselect']);
187-
$client->expects($this->once())
188-
->method('status')
189-
->with($this->equalTo([]))
190-
->willReturn([]);
191-
$folders[0]->expects($this->never())
192-
->method('setStatus');
193-
194-
$this->mapper->getFoldersStatus($folders, $client);
195-
}
196-
197140
public function testGetFoldersStatusAsObject() {
198141
$client = $this->createMock(Horde_Imap_Client_Socket::class);
199142
$client->expects($this->once())

‎tests/Unit/Service/MailManagerTest.php‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,9 +141,6 @@ public function testCreateFolder() {
141141
->method('createFolder')
142142
->with($this->equalTo($client), $this->equalTo($account), $this->equalTo('new'))
143143
->willReturn($folder);
144-
$this->folderMapper->expects($this->once())
145-
->method('getFoldersStatus')
146-
->with($this->equalTo([$folder]));
147144
$this->folderMapper->expects($this->once())
148145
->method('detectFolderSpecialUse')
149146
->with($this->equalTo([$folder]));

0 commit comments

Comments
 (0)