Skip to content

Commit

Permalink
fixup! perf: reduce number of avatar requests
Browse files Browse the repository at this point in the history
Signed-off-by: Hamza Mahjoubi <[email protected]>
  • Loading branch information
hamza221 committed Dec 19, 2024
1 parent 147633a commit 32b5995
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 19 deletions.
4 changes: 3 additions & 1 deletion lib/Contracts/IMailSearch.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ public function findMessage(Account $account,
* @param string|null $filter
* @param int|null $cursor
* @param int|null $limit
* @param string|null $userId
*
* @return Message[]
*
Expand All @@ -47,7 +48,8 @@ public function findMessages(Account $account,
string $sortOrder,
?string $filter,
?int $cursor,
?int $limit): array;
?int $limit,
?string $userId): array;

/**
* @param IUser $user
Expand Down
3 changes: 2 additions & 1 deletion lib/Controller/MessagesController.php
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,8 @@ public function index(int $mailboxId,
$order,
$filter === '' ? null : $filter,
$cursor,
$limit
$limit,
$this->currentUserId

Check warning on line 156 in lib/Controller/MessagesController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/MessagesController.php#L155-L156

Added lines #L155 - L156 were not covered by tests
)
);
}
Expand Down
18 changes: 6 additions & 12 deletions lib/IMAP/PreviewEnhancer.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,29 +38,24 @@ class PreviewEnhancer {
/** @var AvatarService */
private $avatarService;

/** @var string|null */
private $userId;

public function __construct(IMAPClientFactory $clientFactory,
ImapMapper $imapMapper,
DbMapper $dbMapper,
LoggerInterface $logger,
AvatarService $avatarService,
string $userId) {
AvatarService $avatarService) {
$this->clientFactory = $clientFactory;
$this->imapMapper = $imapMapper;
$this->mapper = $dbMapper;
$this->logger = $logger;
$this->avatarService = $avatarService;

Check warning on line 50 in lib/IMAP/PreviewEnhancer.php

View check run for this annotation

Codecov / codecov/patch

lib/IMAP/PreviewEnhancer.php#L50

Added line #L50 was not covered by tests
$this->userId = $userId;
}

/**
* @param Message[] $messages
*
* @return Message[]
*/
public function process(Account $account, Mailbox $mailbox, array $messages, string $mode = 'sync'): array {
public function process(Account $account, Mailbox $mailbox, array $messages, bool $preLoadAvatars = false, ?string $userId): array {

Check warning on line 58 in lib/IMAP/PreviewEnhancer.php

View check run for this annotation

Codecov / codecov/patch

lib/IMAP/PreviewEnhancer.php#L58

Added line #L58 was not covered by tests
$needAnalyze = array_reduce($messages, static function (array $carry, Message $message) {
if ($message->getStructureAnalyzed()) {
// Nothing to do
Expand All @@ -70,12 +65,11 @@ public function process(Account $account, Mailbox $mailbox, array $messages, str
return array_merge($carry, [$message->getUid()]);
}, []);

// If we are in the API call, we need to fetch the avatar for the sender
if ($mode === 'apiCall') {
if ($preLoadAvatars) {
foreach ($messages as $message) {
$from = $message->getFrom()->first() ;
if ($message->getAvatar() === null && $from !== null && $from->getEmail() !== null && $this->userId !== null) {
$avatar = $this->avatarService->getAvatar($from->getEmail(), $this->userId);
$from = $message->getFrom()->first();
if ($message->getAvatar() === null && $from !== null && $from->getEmail() !== null && $userId !== null) {
$avatar = $this->avatarService->getAvatar($from->getEmail(), $userId, true);
$message->setAvatar($avatar);

Check warning on line 73 in lib/IMAP/PreviewEnhancer.php

View check run for this annotation

Codecov / codecov/patch

lib/IMAP/PreviewEnhancer.php#L68-L73

Added lines #L68 - L73 were not covered by tests
}
}
Expand Down
4 changes: 2 additions & 2 deletions lib/Service/AvatarService.php
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,12 @@ private function hasAllowedMime(Avatar $avatar): bool {
* @param string $uid
* @return Avatar|null
*/
public function getAvatar(string $email, string $uid): ?Avatar {
public function getAvatar(string $email, string $uid, bool $cachedOnly = false): ?Avatar {
$cachedAvatar = $this->cache->get($email, $uid);
if ($cachedAvatar) {
return $cachedAvatar;
}
if ($cachedAvatar === false) {
if ($cachedAvatar === false || $cachedOnly) {
// We know there is no avatar
return null;
}
Expand Down
6 changes: 4 additions & 2 deletions lib/Service/Search/MailSearch.php
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ public function findMessages(Account $account,
string $sortOrder,
?string $filter,
?int $cursor,
?int $limit): array {
?int $limit,
?string $userId): array {
if ($mailbox->hasLocks($this->timeFactory->getTime())) {
throw MailboxLockedException::from($mailbox);
}
Expand Down Expand Up @@ -119,7 +120,8 @@ public function findMessages(Account $account,
$this->getIdsLocally($account, $mailbox, $query, $sortOrder, $limit),
$sortOrder,
),
'apiCall'
true,
$userId

Check warning on line 124 in lib/Service/Search/MailSearch.php

View check run for this annotation

Codecov / codecov/patch

lib/Service/Search/MailSearch.php#L122-L124

Added lines #L122 - L124 were not covered by tests
);
}

Expand Down
10 changes: 9 additions & 1 deletion src/components/Avatar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
<script>
import NcAvatar from '@nextcloud/vue/dist/Components/NcAvatar.js'
import { generateUrl } from '@nextcloud/router'
import { fetchAvatarUrlMemoized } from '../service/AvatarService.js'
import logger from '../logger.js'

export default {
Expand Down Expand Up @@ -65,8 +66,15 @@ export default {
email: this.email,
})
: this.avatar.url
} else {
if (this.email !== '') {
try {
this.avatarUrl = await fetchAvatarUrlMemoized(this.email)
} catch {
logger.debug('Could not fetch avatar', { email: this.email })
}
}
}
logger.debug('Could not fetch avatar', { email: this.email })
this.loading = false
},
}
Expand Down

0 comments on commit 32b5995

Please sign in to comment.