From 825736a6c8e1dc5cd6a5c6a5a799f657bf5784e6 Mon Sep 17 00:00:00 2001 From: Glandos Date: Tue, 1 Nov 2022 16:15:30 +0100 Subject: [PATCH 1/6] List preview directory only once getCachedPreview used to call `getFile`, and this calls `getDirectoryListing` (or underlying function that list directory) to find the file. This was done for every preview spec. Now, this is done only once at the beginning of the loop, and the array is just iterated when needed to find the correct entry. Signed-off-by: Glandos --- lib/private/Preview/Generator.php | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/lib/private/Preview/Generator.php b/lib/private/Preview/Generator.php index a49cc8e522ec8..5cf19ccc7d9b8 100644 --- a/lib/private/Preview/Generator.php +++ b/lib/private/Preview/Generator.php @@ -169,6 +169,8 @@ public function generatePreviews(File $file, array $specifications, $mimeType = [$maxWidth, $maxHeight] = $this->getPreviewSize($maxPreview, $previewVersion); $preview = null; + // List every existing preview first instead of trying to find them one by one + $previewFiles = $previewFolder->getDirectoryListing(); foreach ($specifications as $specification) { $width = $specification['width'] ?? -1; @@ -195,7 +197,7 @@ public function generatePreviews(File $file, array $specifications, $mimeType = // Try to get a cached preview. Else generate (and store) one try { try { - $preview = $this->getCachedPreview($previewFolder, $width, $height, $crop, $maxPreview->getMimeType(), $previewVersion); + $preview = $this->getCachedPreview($previewFiles, $width, $height, $crop, $maxPreview->getMimeType(), $previewVersion); } catch (NotFoundException $e) { if (!$this->previewManager->isMimeSupported($mimeType)) { throw new NotFoundException(); @@ -206,6 +208,8 @@ public function generatePreviews(File $file, array $specifications, $mimeType = } $preview = $this->generatePreview($previewFolder, $maxPreviewImage, $width, $height, $crop, $maxWidth, $maxHeight, $previewVersion); + // New file, augment our array + $previewFiles[] = $preview; } } catch (\InvalidArgumentException $e) { throw new NotFoundException("", 0, $e); @@ -545,7 +549,7 @@ private function generatePreview(ISimpleFolder $previewFolder, IImage $maxPrevie } /** - * @param ISimpleFolder $previewFolder + * @param array $files Array of FileInfo, as the result of getDirectoryListing() * @param int $width * @param int $height * @param bool $crop @@ -555,10 +559,14 @@ private function generatePreview(ISimpleFolder $previewFolder, IImage $maxPrevie * * @throws NotFoundException */ - private function getCachedPreview(ISimpleFolder $previewFolder, $width, $height, $crop, $mimeType, $prefix) { + private function getCachedPreview($files, $width, $height, $crop, $mimeType, $prefix) { $path = $this->generatePath($width, $height, $crop, $mimeType, $prefix); - - return $previewFolder->getFile($path); + foreach($files as $file) { + if ($file->getName() === $path) { + return $file; + } + } + throw new NotFoundException(); } /** From ba6e6e5c97fcb38fc43ff675aaf73d40575eb7c8 Mon Sep 17 00:00:00 2001 From: Glandos Date: Tue, 1 Nov 2022 22:09:50 +0100 Subject: [PATCH 2/6] php-cs-fix Signed-off-by: Glandos --- lib/private/Preview/Generator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/Preview/Generator.php b/lib/private/Preview/Generator.php index 5cf19ccc7d9b8..8cbbdd7a87af4 100644 --- a/lib/private/Preview/Generator.php +++ b/lib/private/Preview/Generator.php @@ -561,7 +561,7 @@ private function generatePreview(ISimpleFolder $previewFolder, IImage $maxPrevie */ private function getCachedPreview($files, $width, $height, $crop, $mimeType, $prefix) { $path = $this->generatePath($width, $height, $crop, $mimeType, $prefix); - foreach($files as $file) { + foreach ($files as $file) { if ($file->getName() === $path) { return $file; } From efa3841e9f579a63f66ba82c7405e15cc6c62dee Mon Sep 17 00:00:00 2001 From: Glandos Date: Tue, 1 Nov 2022 22:16:56 +0100 Subject: [PATCH 3/6] Ensure max preview image is not null Signed-off-by: Glandos --- lib/private/Preview/Generator.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/private/Preview/Generator.php b/lib/private/Preview/Generator.php index 8cbbdd7a87af4..b4430eebbffe7 100644 --- a/lib/private/Preview/Generator.php +++ b/lib/private/Preview/Generator.php @@ -206,6 +206,9 @@ public function generatePreviews(File $file, array $specifications, $mimeType = if ($maxPreviewImage === null) { $maxPreviewImage = $this->helper->getImage($maxPreview); } + if ($maxPreviewImage === null) { + throw new NotFoundException(); + } $preview = $this->generatePreview($previewFolder, $maxPreviewImage, $width, $height, $crop, $maxWidth, $maxHeight, $previewVersion); // New file, augment our array From 690648df3b2cfeb2c000b980e055c1672948d475 Mon Sep 17 00:00:00 2001 From: Glandos Date: Wed, 2 Nov 2022 21:35:16 +0100 Subject: [PATCH 4/6] improve parameter doc Signed-off-by: Glandos --- lib/private/Preview/Generator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/Preview/Generator.php b/lib/private/Preview/Generator.php index b4430eebbffe7..46de042004890 100644 --- a/lib/private/Preview/Generator.php +++ b/lib/private/Preview/Generator.php @@ -552,7 +552,7 @@ private function generatePreview(ISimpleFolder $previewFolder, IImage $maxPrevie } /** - * @param array $files Array of FileInfo, as the result of getDirectoryListing() + * @param ISimpleFile[] $files Array of FileInfo, as the result of getDirectoryListing() * @param int $width * @param int $height * @param bool $crop From c0796c90b8515788ea3419714bbd2624edd539ec Mon Sep 17 00:00:00 2001 From: Glandos Date: Wed, 2 Nov 2022 21:35:54 +0100 Subject: [PATCH 5/6] Revert 0e49b40 Signed-off-by: Glandos --- lib/private/Preview/Generator.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/private/Preview/Generator.php b/lib/private/Preview/Generator.php index 46de042004890..709341a621c1b 100644 --- a/lib/private/Preview/Generator.php +++ b/lib/private/Preview/Generator.php @@ -206,9 +206,6 @@ public function generatePreviews(File $file, array $specifications, $mimeType = if ($maxPreviewImage === null) { $maxPreviewImage = $this->helper->getImage($maxPreview); } - if ($maxPreviewImage === null) { - throw new NotFoundException(); - } $preview = $this->generatePreview($previewFolder, $maxPreviewImage, $width, $height, $crop, $maxWidth, $maxHeight, $previewVersion); // New file, augment our array From 4608afb39453e51be5d568d6fee03ddd67ab1b18 Mon Sep 17 00:00:00 2001 From: Glandos Date: Thu, 3 Nov 2022 23:27:55 +0100 Subject: [PATCH 6/6] adapt test to new workflow getDirectoryListing should return all files getFile is not called anymore Signed-off-by: Glandos --- tests/lib/Preview/GeneratorTest.php | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/tests/lib/Preview/GeneratorTest.php b/tests/lib/Preview/GeneratorTest.php index 0dec1aaafa8fa..8199fe4102abc 100644 --- a/tests/lib/Preview/GeneratorTest.php +++ b/tests/lib/Preview/GeneratorTest.php @@ -106,15 +106,12 @@ public function testGetCachedPreview() { $maxPreview->method('getMimeType') ->willReturn('image/png'); - $previewFolder->method('getDirectoryListing') - ->willReturn([$maxPreview]); - $previewFile = $this->createMock(ISimpleFile::class); $previewFile->method('getSize')->willReturn(1000); + $previewFile->method('getName')->willReturn('256-256.png'); - $previewFolder->method('getFile') - ->with($this->equalTo('256-256.png')) - ->willReturn($previewFile); + $previewFolder->method('getDirectoryListing') + ->willReturn([$maxPreview, $previewFile]); $this->legacyEventDispatcher->expects($this->once()) ->method('dispatch') @@ -340,14 +337,12 @@ public function testReturnCachedPreviewsWithoutCheckingSupportedMimetype() { $maxPreview->method('getMimeType') ->willReturn('image/png'); - $previewFolder->method('getDirectoryListing') - ->willReturn([$maxPreview]); - $preview = $this->createMock(ISimpleFile::class); $preview->method('getSize')->willReturn(1000); - $previewFolder->method('getFile') - ->with($this->equalTo('1024-512-crop.png')) - ->willReturn($preview); + $preview->method('getName')->willReturn('1024-512-crop.png'); + + $previewFolder->method('getDirectoryListing') + ->willReturn([$maxPreview, $preview]); $this->previewManager->expects($this->never()) ->method('isMimeSupported');