fixup! perf: reduce number of avatar requests

Signed-off-by: Hamza Mahjoubi <[email protected]>
nextcloud · Jan 12, 2025 · 5dce0d1 · 5dce0d1
1 parent 6f3086e
commit 5dce0d1
Show file tree

Hide file tree

Showing 8 changed files with 46 additions and 9 deletions.
diff --git a/lib/Contracts/IAvatarService.php b/lib/Contracts/IAvatarService.php
@@ -17,9 +17,10 @@ interface IAvatarService {
 	 *
 	 * @param string $email
 	 * @param string $uid
-	 * @return Avatar|null
+	 * @param bool $cachedOnly
+	 * @return Avatar|null|false the avatar if found, false if $cachedOnly is true and no value cached and null if not found
 	 */
-	public function getAvatar(string $email, string $uid): ?Avatar;
+	public function getAvatar(string $email, string $uid, bool $cachedOnly = false): mixed;
 
 	/**
 	 * @param string $email

diff --git a/lib/Db/Message.php b/lib/Db/Message.php
@@ -139,6 +139,10 @@ class Message extends Entity implements JsonSerializable {
 
 	/** @var Avatar|null */
 	private $avatar;
+
+	/** @var bool*/
+	private $fetchAvatarFromClient = false;
+
 	public function __construct() {
 		$this->from = new AddressList([]);
 		$this->to = new AddressList([]);
@@ -297,6 +301,13 @@ public function setAvatar(?Avatar $avatar): void {
 		$this->avatar = $avatar;
 	}
 
+	/**
+	 * @param bool $fetchAvatarFromClient
+	 */
+	public function setFetchAvatarFromClient(bool $fetchAvatarFromClient): void {
+		$this->fetchAvatarFromClient = $fetchAvatarFromClient;
+	}
+
 	/**
 	 * @return Avatar
 	 */
@@ -349,6 +360,7 @@ static function (Tag $tag) {
 			'encrypted' => ($this->isEncrypted() === true),
 			'mentionsMe' => $this->getMentionsMe(),
 			'avatar' => $this->avatar ? $this->avatar->jsonSerialize() : null,
+			'fetchAvatarFromClient' => $this->fetchAvatarFromClient,
 		];
 	}
 }
diff --git a/lib/IMAP/PreviewEnhancer.php b/lib/IMAP/PreviewEnhancer.php
@@ -16,6 +16,7 @@
 use OCA\Mail\Db\MessageMapper as DbMapper;
 use OCA\Mail\IMAP\MessageMapper as ImapMapper;
 use OCA\Mail\Service\AvatarService;
+use OCA\Mail\Service\Avatar\Avatar;
 use Psr\Log\LoggerInterface;
 use function array_key_exists;
 use function array_map;
@@ -70,7 +71,13 @@ public function process(Account $account, Mailbox $mailbox, array $messages, boo
 				$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);
+					if ($avatar === false) {
+						$message->setFetchAvatarFromClient(true);
+					}
+					if ($avatar instanceof Avatar) {
+						$message->setAvatar($avatar);
+					}
+
 				}
 			}
 		}

diff --git a/lib/Service/AvatarService.php b/lib/Service/AvatarService.php
@@ -89,18 +89,24 @@ private function hasAllowedMime(Avatar $avatar): bool {
 	/**
 	 * @param string $email
 	 * @param string $uid
-	 * @return Avatar|null
+	 * @param bool $cachedOnly
+	 * @return Avatar|null|false
 	 */
-	public function getAvatar(string $email, string $uid, bool $cachedOnly = false): ?Avatar {
+	public function getAvatar(string $email, string $uid, bool $cachedOnly = false): mixed{
 		$cachedAvatar = $this->cache->get($email, $uid);
 		if ($cachedAvatar) {
 			return $cachedAvatar;
 		}
-		if ($cachedAvatar === false || $cachedOnly) {
+		if ($cachedAvatar === false) {
 			// We know there is no avatar
 			return null;
 		}
 
+		if($cachedOnly){
+			// We want to fetch the avatar from the frontend
+			return false;
+		}
+
 		$avatar = $this->source->fetch($email, $this->avatarFactory, $this->externalAvatarsAllowed($uid));
 		if (is_null($avatar) || !$this->hasAllowedMime($avatar)) {
 			// Cannot locate any avatar -> nothing to do here

diff --git a/src/components/Avatar.vue b/src/components/Avatar.vue
@@ -35,6 +35,10 @@ export default {
 			type: Object,
 			default: null,
 		},
+		fetchAvatar: {
+			type: Boolean,
+			default: false,
+		},
 		email: {
 			type: String,
 			required: true,
@@ -66,7 +70,7 @@ export default {
 					email: this.email,
 				})
 				: this.avatar.url
-		} else {
+		} else if (this.fetchAvatar) {
 			if (this.email !== '') {
 				try {
 					this.avatarUrl = await fetchAvatarUrlMemoized(this.email)

diff --git a/src/components/Envelope.vue b/src/components/Envelope.vue
@@ -52,7 +52,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="data.avatar" />
+					<Avatar :display-name="addresses"
+						:email="avatarEmail"
+						:fetch-avatar="data.fetchAvatarFromClient"
+						:avatar="data.avatar" />
 				</template>
 			</div>
 		</template>

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

diff --git a/src/components/ThreadEnvelope.vue b/src/components/ThreadEnvelope.vue
@@ -26,6 +26,7 @@
 					:display-name="envelope.from[0].label"
 					:disable-tooltip="true"
 					:size="40"
+					:fetch-avatar="data.fetchAvatarFromClient"
 					:avatar="envelope.avatar"
 					class="envelope__header__avatar-avatar" />
 				<div v-if="isImportant"