perf: reduce number of avatar requests

Signed-off-by: Hamza Mahjoubi <hamzamahjoubi221@gmail.com>
This commit is contained in:
Hamza Mahjoubi
2024-12-12 19:41:03 +07:00
committed by Hamza
parent e725c5f257
commit 9e08477c5c
14 changed files with 218 additions and 12 deletions

View File

@ -27,4 +27,6 @@ interface IAvatarService {
* @return array|null image data
*/
public function getAvatarImage(string $email, string $uid);
public function getCachedAvatar(string $email, string $uid): Avatar|false|null;
}

View File

@ -39,6 +39,7 @@ interface IMailSearch {
* @param string|null $filter
* @param int|null $cursor
* @param int|null $limit
* @param string|null $userId
* @param string|null $view
*
* @return Message[]
@ -52,6 +53,7 @@ interface IMailSearch {
?string $filter,
?int $cursor,
?int $limit,
?string $userId,
?string $view): array;
/**

View File

@ -156,6 +156,7 @@ class MessagesController extends Controller {
$filter === '' ? null : $filter,
$cursor,
$limit,
$this->currentUserId,
$view
);
return new JSONResponse(

View File

@ -12,6 +12,7 @@ namespace OCA\Mail\Db;
use Horde_Mail_Rfc822_Identification;
use JsonSerializable;
use OCA\Mail\AddressList;
use OCA\Mail\Service\Avatar\Avatar;
use OCP\AppFramework\Db\Entity;
use ReturnTypeWillChange;
use function in_array;
@ -136,6 +137,12 @@ class Message extends Entity implements JsonSerializable {
/** @var Tag[] */
private $tags = [];
/** @var Avatar|null */
private $avatar;
/** @var bool */
private $fetchAvatarFromClient = false;
public function __construct() {
$this->from = new AddressList([]);
$this->to = new AddressList([]);
@ -286,6 +293,24 @@ class Message extends Entity implements JsonSerializable {
);
}
}
/**
* @param Avatar|null $avatar
* @return void
*/
public function setAvatar(?Avatar $avatar): void {
$this->avatar = $avatar;
}
public function setFetchAvatarFromClient(bool $fetchAvatarFromClient): void {
$this->fetchAvatarFromClient = $fetchAvatarFromClient;
}
/**
* @return ?Avatar
*/
public function getAvatar(): ?Avatar {
return $this->avatar;
}
#[\Override]
#[ReturnTypeWillChange]
@ -332,6 +357,8 @@ class Message extends Entity implements JsonSerializable {
'summary' => $this->getSummary(),
'encrypted' => ($this->isEncrypted() === true),
'mentionsMe' => $this->getMentionsMe(),
'avatar' => $this->avatar?->jsonSerialize(),
'fetchAvatarFromClient' => $this->fetchAvatarFromClient,
];
}
}

View File

@ -15,6 +15,8 @@ use OCA\Mail\Db\Mailbox;
use OCA\Mail\Db\Message;
use OCA\Mail\Db\MessageMapper as DbMapper;
use OCA\Mail\IMAP\MessageMapper as ImapMapper;
use OCA\Mail\Service\Avatar\Avatar;
use OCA\Mail\Service\AvatarService;
use Psr\Log\LoggerInterface;
use function array_key_exists;
use function array_map;
@ -34,14 +36,19 @@ class PreviewEnhancer {
/** @var LoggerInterface */
private $logger;
/** @var AvatarService */
private $avatarService;
public function __construct(IMAPClientFactory $clientFactory,
ImapMapper $imapMapper,
DbMapper $dbMapper,
LoggerInterface $logger) {
LoggerInterface $logger,
AvatarService $avatarService) {
$this->clientFactory = $clientFactory;
$this->imapMapper = $imapMapper;
$this->mapper = $dbMapper;
$this->logger = $logger;
$this->avatarService = $avatarService;
}
/**
@ -49,7 +56,7 @@ class PreviewEnhancer {
*
* @return Message[]
*/
public function process(Account $account, Mailbox $mailbox, array $messages): array {
public function process(Account $account, Mailbox $mailbox, array $messages, bool $preLoadAvatars = false, ?string $userId = null): array {
$needAnalyze = array_reduce($messages, static function (array $carry, Message $message) {
if ($message->getStructureAnalyzed()) {
// Nothing to do
@ -59,6 +66,22 @@ class PreviewEnhancer {
return array_merge($carry, [$message->getUid()]);
}, []);
if ($preLoadAvatars) {
foreach ($messages as $message) {
$from = $message->getFrom()->first();
if ($message->getAvatar() === null && $from !== null && $from->getEmail() !== null && $userId !== null) {
$avatar = $this->avatarService->getCachedAvatar($from->getEmail(), $userId);
if ($avatar === null) {
$message->setFetchAvatarFromClient(true);
}
if ($avatar instanceof Avatar) {
$message->setAvatar($avatar);
}
}
}
}
if ($needAnalyze === []) {
// Nothing to enhance
return $messages;

View File

@ -86,6 +86,10 @@ class AvatarService implements IAvatarService {
}
}
public function getCachedAvatar(string $email, string $uid): Avatar|false|null {
return $this->cache->get($email, $uid);
}
/**
* @param string $email
* @param string $uid

View File

@ -95,6 +95,7 @@ class MailSearch implements IMailSearch {
?string $filter,
?int $cursor,
?int $limit,
?string $userId,
?string $view): array {
if ($mailbox->hasLocks($this->timeFactory->getTime())) {
throw MailboxLockedException::from($mailbox);
@ -125,7 +126,9 @@ class MailSearch implements IMailSearch {
$this->messageMapper->findByIds($account->getUserId(),
$this->getIdsLocally($account, $mailbox, $query, $sortOrder, $limit),
$sortOrder,
)
),
true,
$userId
);
}

View File

@ -17,6 +17,7 @@
<script>
import NcAvatar from '@nextcloud/vue/components/NcAvatar'
import { generateUrl } from '@nextcloud/router'
import { fetchAvatarUrlMemoized } from '../service/AvatarService.js'
import logger from '../logger.js'
@ -30,6 +31,14 @@ export default {
type: String,
required: true,
},
avatar: {
type: Object,
default: null,
},
fetchAvatar: {
type: Boolean,
default: false,
},
email: {
type: String,
required: true,
@ -55,14 +64,21 @@ export default {
},
},
async mounted() {
if (this.email !== '') {
try {
this.avatarUrl = await fetchAvatarUrlMemoized(this.email)
} catch {
logger.debug('Could not fetch avatar', { email: this.email })
if (this.avatar) {
this.avatarUrl = this.avatar.isExternal
? generateUrl('/apps/mail/api/avatars/image/{email}', {
email: this.email,
})
: this.avatar.url
} else if (this.fetchAvatar) {
if (this.email !== '') {
try {
this.avatarUrl = await fetchAvatarUrlMemoized(this.email)
} catch {
logger.debug('Could not fetch avatar', { email: this.email })
}
}
}
this.loading = false
},
}

View File

@ -53,7 +53,10 @@
<CheckIcon :size="40" class="check-icon" :class="{ 'app-content-list-item-avatar-selected': selected }" />
</template>
<template v-else>
<Avatar :display-name="addresses" :email="avatarEmail" />
<Avatar :display-name="addresses"
:email="avatarEmail"
:fetch-avatar="data.fetchAvatarFromClient"
:avatar="data.avatar" />
</template>
</div>
</template>

View File

@ -10,7 +10,10 @@
:details="details"
@click="openModal">
<template #icon>
<Avatar :display-name="avatarDisplayName" :email="avatarEmail" />
<Avatar :display-name="avatarDisplayName"
:email="avatarEmail"
:fetch-avatar="data.fetchAvatarFromClient"
:avatar="message.avatar" />
</template>
<template #subname>
{{ subjectForSubtitle }}

View File

@ -26,6 +26,8 @@
:display-name="envelope.from[0].label"
:disable-tooltip="true"
:size="40"
:fetch-avatar="envelope.fetchAvatarFromClient"
:avatar="envelope.avatar"
class="envelope__header__avatar-avatar" />
<div v-if="isImportant"
class="app-content-list-item-star icon-important"

View File

@ -11,6 +11,7 @@ namespace OCA\Mail\Tests\Integration\Db;
use ChristophWurst\Nextcloud\Testing\TestCase;
use OCA\Mail\Db\Message;
use OCA\Mail\Service\Avatar\Avatar;
class MessageTest extends TestCase {
protected function setUp(): void {
@ -109,4 +110,25 @@ class MessageTest extends TestCase {
$this->assertEquals($expected, $message->getThreadRootId());
$this->assertNull($message->getInReplyTo());
}
public function testSetAvatar(): void {
$expected = new Avatar(
'http://example.com/avatar.png',
'image/png',
true
);
$message = new Message();
$message->setAvatar($expected);
$this->assertEquals($expected, $message->getAvatar());
}
public function testSetFetchAvatarFromClient(): void {
$message = new Message();
$message->setFetchAvatarFromClient(true);
$this->assertTrue($message->jsonSerialize()['fetchAvatarFromClient']);
}
}

View File

@ -0,0 +1,93 @@
<?php
declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace Unit\IMAP;
use ChristophWurst\Nextcloud\Testing\TestCase;
use OCA\Mail\Address;
use OCA\Mail\AddressList;
use OCA\Mail\Db\Message;
use OCA\Mail\Db\MessageMapper as DbMapper;
use OCA\Mail\IMAP\IMAPClientFactory;
use OCA\Mail\IMAP\MessageMapper as ImapMapper;
use OCA\Mail\IMAP\PreviewEnhancer;
use OCA\Mail\Service\Avatar\Avatar;
use OCA\Mail\Service\AvatarService;
use PHPUnit\Framework\MockObject\MockObject;
use Psr\Log\LoggerInterface;
class PreviewEnhancerTest extends TestCase {
/** @var IMAPClientFactory|MockObject */
private $imapClientFactory;
/** @var ImapMapper|MockObject */
private $imapMapper;
/** @var DbMapper|MockObject */
private $dbMapper;
/** @var LoggerInterface|MockObject */
private $logger;
/** @var AvatarService|MockObject */
private $avatarService;
/** @var PreviewEnhancer */
private $previewEnhancer;
protected function setUp(): void {
parent::setUp();
$this->imapClientFactory = $this->createMock(IMAPClientFactory::class);
$this->imapMapper = $this->createMock(ImapMapper::class);
$this->dbMapper = $this->createMock(DbMapper::class);
$this->logger = $this->createMock(LoggerInterface::class);
$this->avatarService = $this->createMock(AvatarService::class);
$this->previewEnhancer = new previewEnhancer($this->imapClientFactory,
$this->imapMapper,
$this->dbMapper,
$this->logger,
$this->avatarService);
}
public function testAvatars(): void {
$message1 = new Message();
$message1->setId(1);
$message1->setStructureAnalyzed(true);
$message1->setFrom(new AddressList([Address::fromRaw('Alice', 'alice@example.com')]));
$message2 = new Message();
$message2->setId(2);
$message2->setStructureAnalyzed(true);
$message2->setFrom(new AddressList([Address::fromRaw('Bob', 'bob@example.com')]));
$messages = [$message1, $message2];
$message2Avatar = new Avatar('example.com', 'image/png', true);
$this->avatarService->expects($this->exactly(2))
->method('getCachedAvatar')
->withConsecutive(
['alice@example.com', 'testuser'],
['bob@example.com', 'testuser']
)
->willReturnOnConsecutiveCalls(
null,
$message2Avatar
);
$this->previewEnhancer->process(
$this->createMock(\OCA\Mail\Account::class),
$this->createMock(\OCA\Mail\Db\Mailbox::class),
$messages,
true,
'testuser'
);
$this->assertTrue($message1->jsonSerialize()['fetchAvatarFromClient']);
$this->assertFalse($message2->jsonSerialize()['fetchAvatarFromClient']);
$this->assertNull($message1->getAvatar());
$this->assertSame($message2Avatar, $message2->getAvatar());
}
}

View File

@ -82,6 +82,7 @@ class MailSearchTest extends TestCase {
null,
null,
null,
null,
null
);
}
@ -99,7 +100,8 @@ class MailSearchTest extends TestCase {
null,
null,
null,
null
null,
null,
);
}
@ -120,6 +122,7 @@ class MailSearchTest extends TestCase {
null,
null,
null,
null,
null
);
@ -160,6 +163,7 @@ class MailSearchTest extends TestCase {
'my search',
null,
null,
null,
null
);
@ -203,6 +207,7 @@ class MailSearchTest extends TestCase {
'my search',
null,
null,
null,
null
);