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 12, 2024
1 parent e76643a commit f3abc06
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 9 deletions.
5 changes: 5 additions & 0 deletions appinfo/routes.php
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,11 @@
'url' => '/api/messages/{messageId}/smartreply',
'verb' => 'GET'
],
[
'name' => 'avatars#url',
'url' => '/api/avatars/url/{email}',
'verb' => 'GET'
],
[
'name' => 'avatars#image',
'url' => '/api/avatars/image/{email}',
Expand Down
37 changes: 37 additions & 0 deletions lib/Controller/AvatarsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,43 @@ public function __construct(string $appName,
$this->uid = $UserId;
}

/**
* @NoAdminRequired
* @NoCSRFRequired
*
* @param string $email
* @return JSONResponse
*/
#[TrapError]
public function url(string $email): JSONResponse {
if (empty($email)) {
return new JSONResponse([], Http::STATUS_BAD_REQUEST);
}

if (!filter_var($email, FILTER_VALIDATE_EMAIL)) {
return new JSONResponse([], Http::STATUS_BAD_REQUEST);
}

$avatar = $this->avatarService->getAvatar($email, $this->uid);
if (is_null($avatar)) {
// No avatar found
$response = new JSONResponse([], Http::STATUS_NOT_FOUND);

// Debounce this a bit
// (cache for one day)
$response->cacheFor(24 * 60 * 60, false, true);

return $response;
}

$response = new JSONResponse($avatar);

// Let the browser cache this for a week
$response->cacheFor(7 * 24 * 60 * 60, false, true);

return $response;
}

/**
* @NoAdminRequired
* @NoCSRFRequired
Expand Down
21 changes: 12 additions & 9 deletions lib/IMAP/PreviewEnhancer.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,21 +38,21 @@ class PreviewEnhancer {
/** @var AvatarService */
private $avatarService;

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

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

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

View check run for this annotation

Codecov / codecov/patch

lib/IMAP/PreviewEnhancer.php#L54-L55

Added lines #L54 - L55 were not covered by tests
}

/**
Expand All @@ -63,9 +63,9 @@ public function __construct(IMAPClientFactory $clientFactory,
public function process(Account $account, Mailbox $mailbox, array $messages): array {
$needAnalyze = array_reduce($messages, function (array $carry, Message $message) {

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

View check run for this annotation

Codecov / codecov/patch

lib/IMAP/PreviewEnhancer.php#L64

Added line #L64 was not covered by tests
if ($message->getStructureAnalyzed()) {
// Nothing to do
if ($message->getAvatar() === null) {
$avatar = $this->avatarService->getAvatar($message->getFrom()->first()->getEmail(), $this->UserId);
// Try fetching the avatar if it's not set
if ($message->getAvatar() === null && $message->getFrom()->first() !== null) {
$avatar = $this->avatarService->getAvatar($message->getFrom()->first()->getEmail(), $this->userId);

Check failure on line 68 in lib/IMAP/PreviewEnhancer.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

PossiblyNullArgument

lib/IMAP/PreviewEnhancer.php:68:48: PossiblyNullArgument: Argument 1 of OCA\Mail\Service\AvatarService::getAvatar cannot be null, possibly null value provided (see https://psalm.dev/078)

Check failure on line 68 in lib/IMAP/PreviewEnhancer.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

PossiblyNullReference

lib/IMAP/PreviewEnhancer.php:68:78: PossiblyNullReference: Cannot call method getEmail on possibly null value (see https://psalm.dev/083)

Check failure on line 68 in lib/IMAP/PreviewEnhancer.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

PossiblyNullArgument

lib/IMAP/PreviewEnhancer.php:68:90: PossiblyNullArgument: Argument 2 of OCA\Mail\Service\AvatarService::getAvatar cannot be null, possibly null value provided (see https://psalm.dev/078)
$message->setAvatar($avatar);

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

View check run for this annotation

Codecov / codecov/patch

lib/IMAP/PreviewEnhancer.php#L67-L69

Added lines #L67 - L69 were not covered by tests
}
return $carry;
Expand Down Expand Up @@ -112,8 +112,11 @@ public function process(Account $account, Mailbox $mailbox, array $messages): ar
$message->setEncrypted($structureData->isEncrypted());
$message->setMentionsMe($structureData->getMentionsMe());

$avatar = $this->avatarService->getAvatar($message->getFrom()->first()->getEmail(), $this->UserId);
$message->setAvatar($avatar);
if ($message->getFrom()->first() !== null) {
$avatar = $this->avatarService->getAvatar($message->getFrom()->first()->getEmail(), $this->userId);

Check failure on line 116 in lib/IMAP/PreviewEnhancer.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

PossiblyNullArgument

lib/IMAP/PreviewEnhancer.php:116:47: PossiblyNullArgument: Argument 1 of OCA\Mail\Service\AvatarService::getAvatar cannot be null, possibly null value provided (see https://psalm.dev/078)

Check failure on line 116 in lib/IMAP/PreviewEnhancer.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

PossiblyNullReference

lib/IMAP/PreviewEnhancer.php:116:77: PossiblyNullReference: Cannot call method getEmail on possibly null value (see https://psalm.dev/083)

Check failure on line 116 in lib/IMAP/PreviewEnhancer.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

PossiblyNullArgument

lib/IMAP/PreviewEnhancer.php:116:89: PossiblyNullArgument: Argument 2 of OCA\Mail\Service\AvatarService::getAvatar cannot be null, possibly null value provided (see https://psalm.dev/078)
$message->setAvatar($avatar);

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

View check run for this annotation

Codecov / codecov/patch

lib/IMAP/PreviewEnhancer.php#L115-L117

Added lines #L115 - L117 were not covered by tests

}

return $message;
}, $messages));
Expand Down
39 changes: 39 additions & 0 deletions src/service/AvatarService.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/**
* SPDX-FileCopyrightText: 2018 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

import memoize from 'lodash/fp/memoize.js'
import Axios from '@nextcloud/axios'
import { generateUrl } from '@nextcloud/router'

export const fetchAvatarUrl = (email) => {
if (email === null) {
return Promise.resolve(undefined)
}

const url = generateUrl('/apps/mail/api/avatars/url/{email}', {
email,
})

return Axios.get(url)
.then((resp) => resp.data)
.then((avatar) => {
if (avatar.isExternal) {
return generateUrl('/apps/mail/api/avatars/image/{email}', {
email,
})
} else {
return avatar.url
}
})
.catch((err) => {
if (err.response.status === 404) {
return undefined
}

return Promise.reject(err)
})
}

export const fetchAvatarUrlMemoized = memoize(fetchAvatarUrl)

0 comments on commit f3abc06

Please sign in to comment.