From 66975ec6a0fe389f79a537f8f302a7e73938e5e3 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 4 Dec 2023 13:48:21 +0100 Subject: [PATCH] Preload the files metadata and cache photos-size Signed-off-by: Joas Schilling --- lib/Chat/Parser/SystemMessage.php | 21 ++-- lib/Controller/ChatController.php | 14 ++- lib/Share/Helper/FilesMetadataCache.php | 103 ++++++++++++++++++++ tests/php/Chat/Parser/SystemMessageTest.php | 42 +++----- tests/php/Controller/ChatControllerTest.php | 5 + 5 files changed, 142 insertions(+), 43 deletions(-) create mode 100644 lib/Share/Helper/FilesMetadataCache.php diff --git a/lib/Chat/Parser/SystemMessage.php b/lib/Chat/Parser/SystemMessage.php index 799d804429e..7c3c607dcf3 100644 --- a/lib/Chat/Parser/SystemMessage.php +++ b/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) { } } diff --git a/lib/Controller/ChatController.php b/lib/Controller/ChatController.php index 18b21c97c8d..6c4570667b1 100644 --- a/lib/Controller/ChatController.php +++ b/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); } } diff --git a/lib/Share/Helper/FilesMetadataCache.php b/lib/Share/Helper/FilesMetadataCache.php new file mode 100644 index 00000000000..587d2a7066c --- /dev/null +++ b/lib/Share/Helper/FilesMetadataCache.php @@ -0,0 +1,103 @@ + + * + * @author Joas Schilling + * + * @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 . + * + */ + +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 */ + protected array $filesSizeData = []; + + public function __construct( + protected IFilesMetadataManager $filesMetadataManager, + ) { + } + + /** + * @param list $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); + } + } + + /** + * @param int $fileId + * @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 ($this->filesSizeData[$fileId] === null) { + 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; + } + + } +} diff --git a/tests/php/Chat/Parser/SystemMessageTest.php b/tests/php/Chat/Parser/SystemMessageTest.php index 740e2b9ac54..1db0029a362 100644 --- a/tests/php/Chat/Parser/SystemMessageTest.php +++ b/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; @@ -40,8 +41,6 @@ use OCP\Files\IRootFolder; use OCP\Files\Node; use OCP\Files\NotFoundException; -use OCP\FilesMetadata\IFilesMetadataManager; -use OCP\FilesMetadata\Model\IFilesMetadata; use OCP\IGroup; use OCP\IGroupManager; use OCP\IL10N; @@ -77,8 +76,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 +94,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 +125,7 @@ protected function getParser(array $methods = []): SystemMessage { $this->rootFolder, $this->cloudIdManager, $this->url, - $this->metadataManager, + $this->filesMetadataCache, ]) ->onlyMethods($methods) ->getMock(); @@ -144,7 +143,7 @@ protected function getParser(array $methods = []): SystemMessage { $this->rootFolder, $this->cloudIdManager, $this->url, - $this->metadataManager, + $this->filesMetadataCache, ); } @@ -624,7 +623,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 +659,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 +679,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 +735,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 +827,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') diff --git a/tests/php/Controller/ChatControllerTest.php b/tests/php/Controller/ChatControllerTest.php index e1120a02150..a5bbe6da6b6 100644 --- a/tests/php/Controller/ChatControllerTest.php +++ b/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,