Preload the files metadata and cache photos-size

Signed-off-by: Joas Schilling <coding@schilljs.com>

lib/Chat/Parser/SystemMessage.php

@@ -34,6 +34,7 @@
 use OCA\Talk\Participant;
 use OCA\Talk\Room;
 use OCA\Talk\Service\ParticipantService;
+use OCA\Talk\Share\Helper\FilesMetadataCache;
 use OCA\Talk\Share\RoomShareProvider;
 use OCP\Comments\IComment;
 use OCP\EventDispatcher\Event;
@@ -44,7 +45,6 @@
 use OCP\Files\Node;
 use OCP\Files\NotFoundException;
 use OCP\FilesMetadata\Exceptions\FilesMetadataNotFoundException;
-use OCP\FilesMetadata\IFilesMetadataManager;
 use OCP\IGroup;
 use OCP\IGroupManager;
 use OCP\IL10N;
@@ -87,7 +87,7 @@ public function __construct(
 		protected IRootFolder $rootFolder,
 		protected ICloudIdManager $cloudIdManager,
 		protected IURLGenerator $url,
-		protected IFilesMetadataManager $metadataManager,
+		protected FilesMetadataCache $metadataCache,
 	) {
 	}
 
@@ -739,19 +739,14 @@ protected function getFileFromShare(?Participant $participant, string $shareId):
 		];
 
 		// If a preview is available, check if we can get the dimensions of the file from the metadata API
-		if ($isPreviewAvailable) {
+		if ($isPreviewAvailable && str_starts_with($node->getMimeType(), 'image/')) {
 			try {
-				$metadata = $this->metadataManager->getMetaData($fileId, false);
-
-				if ($metadata->hasKey('photos-size')) {
-					$sizeMetadata = $metadata->getArray('photos-size');
-
-					if (isset($sizeMetadata['width']) && isset($sizeMetadata['height'])) {
-						$data['width'] = $sizeMetadata['width'];
-						$data['height'] = $sizeMetadata['height'];
-					}
+				$sizeMetadata = $this->metadataCache->getMetadataPhotosSizeForFileId($fileId);
+				if (isset($sizeMetadata['width'], $sizeMetadata['height'])) {
+					$data['width'] = $sizeMetadata['width'];
+					$data['height'] = $sizeMetadata['height'];
 				}
-			} catch (FilesMetadataNotFoundException $e) {
+			} catch (FilesMetadataNotFoundException) {
 			}
 		}
 

lib/Controller/ChatController.php

@@ -51,6 +51,7 @@
 use OCA\Talk\Service\ParticipantService;
 use OCA\Talk\Service\ReminderService;
 use OCA\Talk\Service\SessionService;
+use OCA\Talk\Share\Helper\FilesMetadataCache;
 use OCA\Talk\Share\RoomShareProvider;
 use OCP\App\IAppManager;
 use OCP\AppFramework\Db\DoesNotExistException;
@@ -73,6 +74,7 @@
 use OCP\RichObjectStrings\IValidator;
 use OCP\Security\ITrustedDomainHelper;
 use OCP\Share\Exceptions\ShareNotFound;
+use OCP\Share\IShare;
 use OCP\User\Events\UserLiveStatusEvent;
 use OCP\UserStatus\IManager as IUserStatusManager;
 use OCP\UserStatus\IUserStatus;
@@ -103,6 +105,7 @@ public function __construct(
 		private GuestManager $guestManager,
 		private MessageParser $messageParser,
 		protected RoomShareProvider $shareProvider,
+		protected FilesMetadataCache $filesMetadataCache,
 		private IManager $autoCompleteManager,
 		private IUserStatusManager $statusManager,
 		protected MatterbridgeManager $matterbridgeManager,
@@ -308,7 +311,8 @@ public function shareObjectToChat(string $objectType, string $objectId, string $
 
 	/*
 	 * Gather share IDs from the comments and preload share definitions
-	 * to avoid separate database query for each individual share.
+	 * and files metadata to avoid separate database query for each
+	 * individual share/node later on.
 	 *
 	 * @param IComment[] $comments
 	 */
@@ -326,10 +330,14 @@ protected function preloadShares(array $comments): void {
 			}
 		}
 		if (!empty($shareIds)) {
-			// Ignore the result for now. Retrieved Share objects will be cached by
+			// Retrieved Share objects will be cached by
 			// the RoomShareProvider and returned from the cache to
 			// the Parser\SystemMessage without additional database queries.
-			$this->shareProvider->getSharesByIds($shareIds);
+			$shares = $this->shareProvider->getSharesByIds($shareIds);
+
+			// Preload files metadata as well
+			$fileIds = array_filter(array_map(static fn (IShare $share) => $share->getNodeId(), $shares));
+			$this->filesMetadataCache->preloadMetadata($fileIds);
 		}
 	}
 

lib/Share/Helper/FilesMetadataCache.php

@@ -0,0 +1,102 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2023 Joas Schilling <coding@schilljs.com>
+ *
+ * @author Joas Schilling <coding@schilljs.com>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCA\Talk\Share\Helper;
+
+use OCP\FilesMetadata\Exceptions\FilesMetadataNotFoundException;
+use OCP\FilesMetadata\Exceptions\FilesMetadataTypeException;
+use OCP\FilesMetadata\IFilesMetadataManager;
+use OCP\FilesMetadata\Model\IFilesMetadata;
+
+class FilesMetadataCache {
+	/** @var array<int, array{width: int, height: int}> */
+	protected array $filesSizeData = [];
+
+	public function __construct(
+		protected IFilesMetadataManager $filesMetadataManager,
+	) {
+	}
+
+	/**
+	 * @param list<int> $fileIds
+	 */
+	public function preloadMetadata(array $fileIds): void {
+		$missingFileIds = array_diff($fileIds, array_keys($this->filesSizeData));
+		if (empty($missingFileIds)) {
+			return;
+		}
+
+		$data = $this->filesMetadataManager->getMetadataForFiles($missingFileIds);
+		foreach ($data as $fileId => $metadata) {
+			$this->cachePhotosSize($fileId, $metadata);
+		}
+	}
+
+	/**
+	 * @return array
+	 * @psalm-return array{width: int, height: int}
+	 * @throws FilesMetadataNotFoundException
+	 */
+	public function getMetadataPhotosSizeForFileId(int $fileId): array {
+		if (!array_key_exists($fileId, $this->filesSizeData)) {
+			try {
+				$this->cachePhotosSize($fileId, $this->filesMetadataManager->getMetadata($fileId, true));
+			} catch (FilesMetadataNotFoundException) {
+				$this->filesSizeData[$fileId] = null;
+			}
+		}
+
+		if (!isset($this->filesSizeData[$fileId])) {
+			return throw new FilesMetadataNotFoundException();
+		}
+
+		return $this->filesSizeData[$fileId];
+	}
+
+	protected function cachePhotosSize(int $fileId, IFilesMetadata $metadata): void {
+		if ($metadata->hasKey('photos-size')) {
+			try {
+				$sizeMetadata = $metadata->getArray('photos-size');
+			} catch (FilesMetadataNotFoundException|FilesMetadataTypeException) {
+				$this->filesSizeData[$fileId] = null;
+				return;
+			}
+
+			if (isset($sizeMetadata['width'], $sizeMetadata['height'])) {
+				$dimensions = [
+					'width' => $sizeMetadata['width'],
+					'height' => $sizeMetadata['height'],
+				];
+				$this->filesSizeData[$fileId] = $dimensions;
+			} else {
+				$this->filesSizeData[$fileId] = null;
+			}
+		} else {
+			$this->filesSizeData[$fileId] = null;
+		}
+
+	}
+}

tests/php/Chat/Parser/SystemMessageTest.php

@@ -32,6 +32,7 @@
 use OCA\Talk\Participant;
 use OCA\Talk\Room;
 use OCA\Talk\Service\ParticipantService;
+use OCA\Talk\Share\Helper\FilesMetadataCache;
 use OCA\Talk\Share\RoomShareProvider;
 use OCP\Comments\IComment;
 use OCP\Federation\ICloudIdManager;
@@ -46,6 +47,7 @@
 use OCP\IGroupManager;
 use OCP\IL10N;
 use OCP\IPreview as IPreviewManager;
+use OCP\IRequest;
 use OCP\IURLGenerator;
 use OCP\IUserManager;
 use OCP\Share\Exceptions\ShareNotFound;
@@ -77,8 +79,8 @@ class SystemMessageTest extends TestCase {
 	protected $url;
 	/** @var ICloudIdManager|MockObject */
 	protected $cloudIdManager;
-	/** @var IFilesMetadataManager|MockObject */
-	protected $metadataManager;
+	/** @var FilesMetadataCache|MockObject */
+	protected $filesMetadataCache;
 	/** @var IL10N|MockObject */
 	protected $l;
 
@@ -95,7 +97,7 @@ public function setUp(): void {
 		$this->rootFolder = $this->createMock(IRootFolder::class);
 		$this->url = $this->createMock(IURLGenerator::class);
 		$this->cloudIdManager = $this->createMock(ICloudIdManager::class);
-		$this->metadataManager = $this->createMock(IFilesMetadataManager::class);
+		$this->filesMetadataCache = $this->createMock(FilesMetadataCache::class);
 		$this->l = $this->createMock(IL10N::class);
 		$this->l->method('t')
 			->willReturnCallback(function ($text, $parameters = []) {
@@ -126,7 +128,7 @@ protected function getParser(array $methods = []): SystemMessage {
 					$this->rootFolder,
 					$this->cloudIdManager,
 					$this->url,
-					$this->metadataManager,
+					$this->filesMetadataCache,
 				])
 				->onlyMethods($methods)
 				->getMock();
@@ -144,7 +146,7 @@ protected function getParser(array $methods = []): SystemMessage {
 			$this->rootFolder,
 			$this->cloudIdManager,
 			$this->url,
-			$this->metadataManager,
+			$this->filesMetadataCache,
 		);
 	}
 
@@ -624,7 +626,7 @@ public function testGetFileFromShareForGuest() {
 			->willReturn('name');
 		$node->expects($this->atLeastOnce())
 			->method('getMimeType')
-			->willReturn('text/plain');
+			->willReturn('image/png');
 		$node->expects($this->once())
 			->method('getSize')
 			->willReturn(65530);
@@ -660,22 +662,11 @@ public function testGetFileFromShareForGuest() {
 			->with($node)
 			->willReturn(true);
 
-		$metadata = $this->createMock(IFilesMetadata::class);
-		$metadata->expects($this->once())
-			->method('hasKey')
-			->with('photos-size')
-			->willReturn(true);
-
-		$metadata->expects($this->once())
-			->method('getArray')
-			->with('photos-size')
+		$this->filesMetadataCache->expects($this->once())
+			->method('getMetadataPhotosSizeForFileId')
+			->with(54)
 			->willReturn(['width' => 1234, 'height' => 4567]);
 
-		$this->metadataManager->expects($this->once())
-			->method('getMetaData')
-			->with(54, false)
-			->willReturn($metadata);
-
 		$participant = $this->createMock(Participant::class);
 		$participant->expects($this->once())
 			->method('isGuest')
@@ -691,7 +682,7 @@ public function testGetFileFromShareForGuest() {
 			'link' => 'absolute-link',
 			'etag' => '1872ade88f3013edeb33decd74a4f947',
 			'permissions' => 27,
-			'mimetype' => 'text/plain',
+			'mimetype' => 'image/png',
 			'preview-available' => 'yes',
 			'width' => 1234,
 			'height' => 4567,
@@ -747,8 +738,8 @@ public function testGetFileFromShareForOwner() {
 			])
 			->willReturn('absolute-link-owner');
 
-		$this->metadataManager->expects($this->never())
-			->method('getMetaData');
+		$this->filesMetadataCache->expects($this->never())
+			->method('getMetadataPhotosSizeForFileId');
 
 		$participant = $this->createMock(Participant::class);
 		$participant->expects($this->once())
@@ -839,8 +830,8 @@ public function testGetFileFromShareForRecipient() {
 			->with($file)
 			->willReturn(false);
 
-		$this->metadataManager->expects($this->never())
-			->method('getMetaData');
+		$this->filesMetadataCache->expects($this->never())
+			->method('getMetadataPhotosSizeForFileId');
 
 		$this->url->expects($this->once())
 			->method('linkToRouteAbsolute')

tests/php/Controller/ChatControllerTest.php

@@ -39,6 +39,7 @@
 use OCA\Talk\Service\ParticipantService;
 use OCA\Talk\Service\ReminderService;
 use OCA\Talk\Service\SessionService;
+use OCA\Talk\Share\Helper\FilesMetadataCache;
 use OCA\Talk\Share\RoomShareProvider;
 use OCP\App\IAppManager;
 use OCP\AppFramework\Http;
@@ -86,6 +87,8 @@ class ChatControllerTest extends TestCase {
 	protected $messageParser;
 	/** @var RoomShareProvider|MockObject */
 	protected $roomShareProvider;
+	/** @var FilesMetadataCache|MockObject */
+	protected $filesMetadataCache;
 	/** @var IManager|MockObject */
 	protected $autoCompleteManager;
 	/** @var IUserStatusManager|MockObject */
@@ -131,6 +134,7 @@ public function setUp(): void {
 		$this->guestManager = $this->createMock(GuestManager::class);
 		$this->messageParser = $this->createMock(MessageParser::class);
 		$this->roomShareProvider = $this->createMock(RoomShareProvider::class);
+		$this->filesMetadataCache = $this->createMock(FilesMetadataCache::class);
 		$this->autoCompleteManager = $this->createMock(IManager::class);
 		$this->statusManager = $this->createMock(IUserStatusManager::class);
 		$this->matterbridgeManager = $this->createMock(MatterbridgeManager::class);
@@ -171,6 +175,7 @@ private function recreateChatController() {
 			$this->guestManager,
 			$this->messageParser,
 			$this->roomShareProvider,
+			$this->filesMetadataCache,
 			$this->autoCompleteManager,
 			$this->statusManager,
 			$this->matterbridgeManager,