From d5469c2b2e22a802de2943d33c2b0dbff1c67643 Mon Sep 17 00:00:00 2001 From: czachor Date: Tue, 20 Oct 2026 13:30:01 +0200 Subject: [PATCH 1/5] perf(listing): skip directory tree walk when tree is disabled indexAction always built the full recursive directory tree, even with tree: false, where it is never displayed. The tree walk is now done only when the tree is enabled; treeData is null otherwise. The JavaScript already initializes the tree only when tree === true. --- Controller/ManagerController.php | 6 ++++-- Tests/Controller/ManagerControllerTest.php | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/Controller/ManagerController.php b/Controller/ManagerController.php index e57712c..f925d55 100644 --- a/Controller/ManagerController.php +++ b/Controller/ManagerController.php @@ -59,8 +59,10 @@ public function indexAction(Request $request, FileTypeService $fileTypeService): } $fileManager = $this->newFileManager($queryParameters); - // Folder search - $directoriesArbo = $this->retrieveSubDirectories($fileManager, $fileManager->getDirName(), \DIRECTORY_SEPARATOR, $fileManager->getBaseName()); + // Folder search, walks the whole directory tree, so only when the tree is displayed + $directoriesArbo = $fileManager->getTree() + ? $this->retrieveSubDirectories($fileManager, $fileManager->getDirName(), \DIRECTORY_SEPARATOR, $fileManager->getBaseName()) + : null; // File search $finderFiles = new Finder(); diff --git a/Tests/Controller/ManagerControllerTest.php b/Tests/Controller/ManagerControllerTest.php index e4e9ab0..75e3d04 100644 --- a/Tests/Controller/ManagerControllerTest.php +++ b/Tests/Controller/ManagerControllerTest.php @@ -104,4 +104,20 @@ public function testTableTr() $this->assertSame($file, $crawler->filter('.list-blk tbody tr')->eq($i)->attr('class')); } } + + public function testJsonWithoutTreeSkipsTreeData() + { + $this->getBackendPage(['conf' => 'default', 'tree' => 0, 'json' => 1]); + $data = json_decode($this->client->getResponse()->getContent(), true); + + $this->assertNull($data['treeData']); + } + + public function testJsonWithTreeHasTreeData() + { + $this->getBackendPage(['conf' => 'default', 'json' => 1]); + $data = json_decode($this->client->getResponse()->getContent(), true); + + $this->assertNotEmpty($data['treeData']); + } } From 37bb269ef0f144704aa1ddb7aecc53072da5b038 Mon Sep 17 00:00:00 2001 From: czachor Date: Tue, 20 Oct 2026 13:31:18 +0200 Subject: [PATCH 2/5] perf(listing): memoize image dimensions without error suppression Sorting by dimension calls getDimension() on every usort comparison, so getimagesize() read each image O(n log n) times. The result is now computed once per file. The image check uses an exact extension list (File::IMAGE_EXTENSIONS) instead of an unanchored regex, and getimagesize() runs through ErrorHandler::call() instead of the @ operator: a corrupt or unreadable image still yields false, without silencing unrelated errors. FileManager::getRegex() builds the "image" type pattern from the same list. --- Helpers/File.php | 23 +++++++- Helpers/FileManager.php | 2 +- Tests/Controller/ManagerControllerTest.php | 6 +++ Tests/Helpers/FileTest.php | 61 ++++++++++++++++++++++ 4 files changed, 89 insertions(+), 3 deletions(-) create mode 100644 Tests/Helpers/FileTest.php diff --git a/Helpers/File.php b/Helpers/File.php index 0ff8846..021ed15 100644 --- a/Helpers/File.php +++ b/Helpers/File.php @@ -3,13 +3,18 @@ namespace Artgris\Bundle\FileManagerBundle\Helpers; use Artgris\Bundle\FileManagerBundle\Service\FileTypeService; +use Symfony\Component\ErrorHandler\ErrorHandler; use Symfony\Component\Finder\SplFileInfo; use Symfony\Contracts\Translation\TranslatorInterface; class File { + public const IMAGE_EXTENSIONS = ['gif', 'png', 'jpg', 'jpeg', 'svg', 'webp']; + private $preview; + private array|false|string|null $dimension = null; + /** * File constructor. */ @@ -19,11 +24,25 @@ public function __construct(private SplFileInfo $file,private TranslatorInterfac } /** + * Memoized: sorting by dimension calls this for every comparison. + * * @return array|false|string */ public function getDimension(): bool|array|string { - return preg_match('/(gif|png|jpe?g|svg|webp)$/i', $this->file->getExtension()) ? - @getimagesize($this->file->getPathname()) : ''; + return $this->dimension ??= $this->readDimension(); + } + + private function readDimension(): array|false|string { + if (!\in_array(strtolower($this->file->getExtension()), self::IMAGE_EXTENSIONS, true)) { + return ''; + } + + try { + // Turns the warning emitted for unreadable or corrupt images into an exception + return ErrorHandler::call('getimagesize', $this->file->getPathname()); + } catch (\ErrorException) { + return false; + } } public function getHTMLDimension(): ?string { diff --git a/Helpers/FileManager.php b/Helpers/FileManager.php index 2c57e92..8d2a74a 100644 --- a/Helpers/FileManager.php +++ b/Helpers/FileManager.php @@ -45,7 +45,7 @@ public function getRegex(): string return match ($this->getType()) { 'media' => '/\.(mp4|ogg|webm)$/i', - 'image' => '/\.(gif|png|jpe?g|svg|webp)$/i', + 'image' => '/\.('.implode('|', File::IMAGE_EXTENSIONS).')$/i', default => '/.+$/i', }; } diff --git a/Tests/Controller/ManagerControllerTest.php b/Tests/Controller/ManagerControllerTest.php index 75e3d04..0134099 100644 --- a/Tests/Controller/ManagerControllerTest.php +++ b/Tests/Controller/ManagerControllerTest.php @@ -120,4 +120,10 @@ public function testJsonWithTreeHasTreeData() $this->assertNotEmpty($data['treeData']); } + + public function testOrderByDimension() + { + $this->getBackendPage(['conf' => 'default', 'orderby' => 'dimension']); + $this->assertSame(200, $this->client->getResponse()->getStatusCode()); + } } diff --git a/Tests/Helpers/FileTest.php b/Tests/Helpers/FileTest.php new file mode 100644 index 0000000..f3135b2 --- /dev/null +++ b/Tests/Helpers/FileTest.php @@ -0,0 +1,61 @@ +dir = sys_get_temp_dir().'/artgris-file-test-'.uniqid(); + mkdir($this->dir); + } + + protected function tearDown(): void + { + array_map('unlink', glob($this->dir.'/*')); + rmdir($this->dir); + } + + public function testDimensionOfImage() + { + // 1x1 transparent PNG + $file = $this->createFile('pixel.PNG', base64_decode('iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=')); + + $this->assertSame([1, 1], \array_slice($file->getDimension(), 0, 2)); + } + + public function testDimensionOfCorruptImageIsFalse() + { + $file = $this->createFile('broken.png', 'not an image'); + + $this->assertFalse($file->getDimension()); + } + + public function testDimensionOfNonImageIsEmpty() + { + $file = $this->createFile('text.txt', 'text'); + + $this->assertSame('', $file->getDimension()); + } + + private function createFile(string $name, string $content): File + { + file_put_contents($this->dir.'/'.$name, $content); + + return new File( + new SplFileInfo($this->dir.'/'.$name, '', $name), + $this->createMock(TranslatorInterface::class), + $this->createMock(FileTypeService::class), + $this->createMock(FileManager::class), + ); + } +} From 876da9abd24a02f140640afcbbdaffdba0ce75bf Mon Sep 17 00:00:00 2001 From: czachor Date: Tue, 20 Oct 2026 13:32:27 +0200 Subject: [PATCH 3/5] refactor(file-type): match extensions exactly instead of by regex fileIcon() matched extensions with unanchored regexes, so any extension ending with a known one matched too (e.g. "xpdf" got the PDF icon). Extensions are now lowercased once and compared against exact lists; images reuse File::IMAGE_EXTENSIONS. "tgz" is listed explicitly, as it was previously matched only through the "gz" suffix. The extension is read with SplFileInfo::getExtension(), which always returns a string, instead of pathinfo(), whose return type is array|string. IMAGE_SIZE gets explicit public visibility. --- Service/FileTypeService.php | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/Service/FileTypeService.php b/Service/FileTypeService.php index b5a9948..26f384d 100644 --- a/Service/FileTypeService.php +++ b/Service/FileTypeService.php @@ -2,13 +2,14 @@ namespace Artgris\Bundle\FileManagerBundle\Service; +use Artgris\Bundle\FileManagerBundle\Helpers\File; use Artgris\Bundle\FileManagerBundle\Helpers\FileManager; use SplFileInfo; use Symfony\Component\Routing\RouterInterface; use Twig\Environment; class FileTypeService { - const IMAGE_SIZE = [ + public const IMAGE_SIZE = [ FileManager::VIEW_LIST => '22', FileManager::VIEW_THUMBNAIL => '100', ]; @@ -72,19 +73,17 @@ public function fileIcon(string $filePath,?string $extension = null, ?int $size $imageTemplate = null; - if (null === $extension) { - $filePathTmp = strtok($filePath, '?'); - $extension = pathinfo($filePathTmp, PATHINFO_EXTENSION); - } + // Strip the query string before reading the extension, then match it case-insensitively + $extension = strtolower($extension ?? (new SplFileInfo(explode('?', $filePath)[0]))->getExtension()); switch (true) { case $this->isYoutubeVideo($filePath): - case preg_match('/(mp4|ogg|webm|avi|wmv|mov)$/i', $extension): + case \in_array($extension, ['mp4', 'ogg', 'webm', 'avi', 'wmv', 'mov'], true): $fa = 'far fa-file-video'; break; - case preg_match('/(mp3|wav)$/i', $extension): + case \in_array($extension, ['mp3', 'wav'], true): $fa = 'far fa-file-audio'; break; - case preg_match('/(gif|png|jpe?g|svg|webp)$/i', $extension): + case \in_array($extension, File::IMAGE_EXTENSIONS, true): $fileName = $filePath; if ($cachebreaker) { @@ -112,19 +111,19 @@ public function fileIcon(string $filePath,?string $extension = null, ?int $size 'html' => $html, 'image' => true, ]; - case preg_match('/(pdf)$/i', $extension): + case 'pdf' === $extension: $fa = 'far fa-file-pdf'; break; - case preg_match('/(docx?)$/i', $extension): + case \in_array($extension, ['doc', 'docx'], true): $fa = 'far fa-file-word'; break; - case preg_match('/(xlsx?|csv)$/i', $extension): + case \in_array($extension, ['xls', 'xlsx', 'csv'], true): $fa = 'far fa-file-excel'; break; - case preg_match('/(pptx?)$/i', $extension): + case \in_array($extension, ['ppt', 'pptx'], true): $fa = 'far fa-file-powerpoint'; break; - case preg_match('/(zip|rar|gz)$/i', $extension): + case \in_array($extension, ['zip', 'rar', 'gz', 'tgz'], true): $fa = 'far fa-file-archive'; break; case filter_var($filePath, FILTER_VALIDATE_URL): From 3fc4d5e756c9bbd8b29ebc43e1eaf03ae3bafb40 Mon Sep 17 00:00:00 2001 From: czachor Date: Tue, 20 Oct 2026 13:33:04 +0200 Subject: [PATCH 4/5] perf(listing): build file preview lazily The File constructor rendered the preview Twig template for every listed entry, whether or not the template displayed it. The preview is now built on the first getPreview() call and reused afterwards; isImage() goes through getPreview(). --- Helpers/File.php | 8 ++++---- Tests/Helpers/FileTest.php | 24 ++++++++++++++++++++++-- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/Helpers/File.php b/Helpers/File.php index 021ed15..46cbc4c 100644 --- a/Helpers/File.php +++ b/Helpers/File.php @@ -11,7 +11,7 @@ class File { public const IMAGE_EXTENSIONS = ['gif', 'png', 'jpg', 'jpeg', 'svg', 'webp']; - private $preview; + private ?array $preview = null; private array|false|string|null $dimension = null; @@ -20,7 +20,6 @@ class File */ public function __construct(private SplFileInfo $file,private TranslatorInterface $translator,private FileTypeService $fileTypeService,private FileManager $fileManager) { - $this->preview = $this->fileTypeService->preview($this->fileManager, $this->file); } /** @@ -87,7 +86,7 @@ public function getAttribut(): ?string { } public function isImage(): bool { - return \array_key_exists('image', $this->preview); + return \array_key_exists('image', $this->getPreview()); } public function getFile(): SplFileInfo { @@ -100,7 +99,8 @@ public function setFile(SplFileInfo $file) :void } public function getPreview(): array { - return $this->preview; + // Lazy: the preview renders a Twig template, so build it only for files that are displayed + return $this->preview ??= $this->fileTypeService->preview($this->fileManager, $this->file); } public function setPreview(array $preview) :void diff --git a/Tests/Helpers/FileTest.php b/Tests/Helpers/FileTest.php index f3135b2..524f2b0 100644 --- a/Tests/Helpers/FileTest.php +++ b/Tests/Helpers/FileTest.php @@ -47,14 +47,34 @@ public function testDimensionOfNonImageIsEmpty() $this->assertSame('', $file->getDimension()); } - private function createFile(string $name, string $content): File + public function testPreviewIsNotBuiltUntilNeeded() + { + $fileTypeService = $this->createMock(FileTypeService::class); + $fileTypeService->expects($this->never())->method('preview'); + + $this->createFile('text.txt', 'text', $fileTypeService)->getDimension(); + } + + public function testPreviewIsBuiltOnce() + { + $fileTypeService = $this->createMock(FileTypeService::class); + $fileTypeService->expects($this->once())->method('preview')->willReturn(['image' => true]); + + $file = $this->createFile('text.txt', 'text', $fileTypeService); + + $this->assertTrue($file->isImage()); + $this->assertSame(['image' => true], $file->getPreview()); + $this->assertSame(['image' => true], $file->getPreview()); + } + + private function createFile(string $name, string $content, ?FileTypeService $fileTypeService = null): File { file_put_contents($this->dir.'/'.$name, $content); return new File( new SplFileInfo($this->dir.'/'.$name, '', $name), $this->createMock(TranslatorInterface::class), - $this->createMock(FileTypeService::class), + $fileTypeService ?? $this->createMock(FileTypeService::class), $this->createMock(FileManager::class), ); } From 10bd47f9fc01af013384595e6ad10c676715df28 Mon Sep 17 00:00:00 2001 From: czachor Date: Tue, 20 Oct 2026 13:35:11 +0200 Subject: [PATCH 5/5] perf(listing): count JSON badge from the file list The JSON response computed the badge with $finderFiles->count(), which iterates the Finder again and rescans the directory. The file list is already built from the same Finder, so its size is used instead. --- Controller/ManagerController.php | 12 ++++++------ Tests/Controller/ManagerControllerTest.php | 4 ++++ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/Controller/ManagerController.php b/Controller/ManagerController.php index f925d55..4882752 100644 --- a/Controller/ManagerController.php +++ b/Controller/ManagerController.php @@ -118,7 +118,7 @@ public function indexAction(Request $request, FileTypeService $fileTypeService): } if ('dimension' === $orderBy) { - usort($fileArray, function (File $a, File $b) { + usort($fileArray, static function (File $a, File $b) { $aDimension = $a->getDimension(); $bDimension = $b->getDimension(); if ($aDimension && !$bDimension) { @@ -149,7 +149,7 @@ public function indexAction(Request $request, FileTypeService $fileTypeService): if ($isJson) { $fileList = $this->renderView('@ArtgrisFileManager/views/_manager_view.html.twig', $parameters); - return new JsonResponse(['data' => $fileList, 'badge' => $finderFiles->count(), 'treeData' => $directoriesArbo]); + return new JsonResponse(['data' => $fileList, 'badge' => \count($fileArray), 'treeData' => $directoriesArbo]); } $parameters['treeData'] = json_encode($directoriesArbo); @@ -215,7 +215,7 @@ public function renameFileAction(Request $request, string $fileName): RedirectRe $fileManager = $this->newFileManager($queryParameters); $newfilePath = $fileManager->getCurrentPath().\DIRECTORY_SEPARATOR.$newfileName; $oldfilePath = realpath($fileManager->getCurrentPath().\DIRECTORY_SEPARATOR.$fileName); - if (0 !== mb_strpos($newfilePath, $fileManager->getCurrentPath())) { + if (!str_starts_with($newfilePath, $fileManager->getCurrentPath())) { $this->addFlash('danger', $this->translator->trans('file.renamed.unauthorized')); } else { $fs = new Filesystem(); @@ -307,7 +307,7 @@ public function deleteAction(Request $request): RedirectResponse { $is_delete = false; foreach ($queryParameters['delete'] as $fileName) { $filePath = realpath($fileManager->getCurrentPath().\DIRECTORY_SEPARATOR.$fileName); - if (0 !== mb_strpos($filePath, $fileManager->getCurrentPath())) { + if (!str_starts_with($filePath, $fileManager->getCurrentPath())) { $this->addFlash('danger', 'file.deleted.danger'); } else { try { @@ -340,7 +340,7 @@ public function deleteAction(Request $request): RedirectResponse { $this->dispatch(FileManagerEvents::POST_DELETE_FOLDER); $queryParameters['route'] = \dirname($fileManager->getRoute()); - if ($queryParameters['route'] == '/') { + if ($queryParameters['route'] === '/') { unset($queryParameters['route']); } @@ -451,7 +451,7 @@ private function newFileManager(array $queryParameters): FileManager { return $this->fileManager; } - protected function dispatch(string $eventName, array $arguments = []) { + protected function dispatch(string $eventName, array $arguments = []): void { $arguments = array_replace([ 'filemanager' => $this->fileManager, ], $arguments); diff --git a/Tests/Controller/ManagerControllerTest.php b/Tests/Controller/ManagerControllerTest.php index 0134099..fed9324 100644 --- a/Tests/Controller/ManagerControllerTest.php +++ b/Tests/Controller/ManagerControllerTest.php @@ -111,6 +111,8 @@ public function testJsonWithoutTreeSkipsTreeData() $data = json_decode($this->client->getResponse()->getContent(), true); $this->assertNull($data['treeData']); + // SubDir and text.txt + $this->assertSame(2, $data['badge']); } public function testJsonWithTreeHasTreeData() @@ -119,6 +121,8 @@ public function testJsonWithTreeHasTreeData() $data = json_decode($this->client->getResponse()->getContent(), true); $this->assertNotEmpty($data['treeData']); + // tree mode lists files only + $this->assertSame(1, $data['badge']); } public function testOrderByDimension()