From 40d8bb453ebca7eacfd3b24f439081ae0969e28e Mon Sep 17 00:00:00 2001 From: Dieter Holvoet Date: Tue, 26 May 2020 11:20:41 +0200 Subject: [PATCH] Get file and folder metadata for media items using a single network call if possible (#5046) Co-Authored-By: Ben Thomson . Fixes #5045. --- modules/system/classes/MediaLibrary.php | 47 ++++++++------ tests/fixtures/media/text.txt | 1 + .../unit/system/classes/MediaLibraryTest.php | 65 +++++++++++++++++++ 3 files changed, 93 insertions(+), 20 deletions(-) create mode 100644 tests/fixtures/media/text.txt diff --git a/modules/system/classes/MediaLibrary.php b/modules/system/classes/MediaLibrary.php index 7c3c1abfe..50a6cc8f9 100644 --- a/modules/system/classes/MediaLibrary.php +++ b/modules/system/classes/MediaLibrary.php @@ -595,14 +595,14 @@ class MediaLibrary } /** - * Initializes a library item from a path and item type. - * @param string $path Specifies the item path relative to the storage disk root. + * Initializes a library item from file metadata and item type. + * @param array $item Specifies the file metadata as returned by the storage adapter. * @param string $itemType Specifies the item type. * @return mixed Returns the MediaLibraryItem object or NULL if the item is not visible. */ - protected function initLibraryItem($path, $itemType) + protected function initLibraryItem($item, $itemType) { - $relativePath = $this->getMediaRelativePath($path); + $relativePath = $this->getMediaRelativePath($item['path']); if (!$this->isVisible($relativePath)) { return; @@ -612,9 +612,11 @@ class MediaLibrary * S3 doesn't allow getting the last modified timestamp for folders, * so this feature is disabled - folders timestamp is always NULL. */ - $lastModified = $itemType == MediaLibraryItem::TYPE_FILE - ? $this->getStorageDisk()->lastModified($path) - : null; + if ($itemType === MediaLibraryItem::TYPE_FILE) { + $lastModified = $item['timestamp'] ?? $this->getStorageDisk()->lastModified($item['path']); + } else { + $lastModified = null; + } /* * The folder size (number of items) doesn't respect filters. That @@ -622,9 +624,11 @@ class MediaLibrary * zero items for a folder that contains files not visible with a * currently applied filter. -ab */ - $size = $itemType == MediaLibraryItem::TYPE_FILE - ? $this->getStorageDisk()->size($path) - : $this->getFolderItemCount($path); + if ($itemType === MediaLibraryItem::TYPE_FILE) { + $size = $item['size'] ?? $this->getStorageDisk()->size($item['path']); + } else { + $size = $this->getFolderItemCount($item['path']); + } $publicUrl = $this->getPathUrl($relativePath); @@ -665,17 +669,20 @@ class MediaLibrary 'folders' => [] ]; - $files = $this->getStorageDisk()->files($fullFolderPath); - foreach ($files as $file) { - if ($libraryItem = $this->initLibraryItem($file, MediaLibraryItem::TYPE_FILE)) { - $result['files'][] = $libraryItem; - } - } + $contents = $this->getStorageDisk()->listContents($fullFolderPath); - $folders = $this->getStorageDisk()->directories($fullFolderPath); - foreach ($folders as $folder) { - if ($libraryItem = $this->initLibraryItem($folder, MediaLibraryItem::TYPE_FOLDER)) { - $result['folders'][] = $libraryItem; + foreach ($contents as $content) { + if ($content['type'] === 'file') { + $type = MediaLibraryItem::TYPE_FILE; + $key = 'files'; + } elseif ($content['type'] === 'dir') { + $type = MediaLibraryItem::TYPE_FOLDER; + $key = 'folders'; + } + + $libraryItem = $this->initLibraryItem($content, $type); + if (!is_null($libraryItem)) { + $result[$key][] = $libraryItem; } } diff --git a/tests/fixtures/media/text.txt b/tests/fixtures/media/text.txt new file mode 100644 index 000000000..e42ff0a79 --- /dev/null +++ b/tests/fixtures/media/text.txt @@ -0,0 +1 @@ +THIS IS A TEXT DOCUMENT diff --git a/tests/unit/system/classes/MediaLibraryTest.php b/tests/unit/system/classes/MediaLibraryTest.php index 24bab1567..35e561594 100644 --- a/tests/unit/system/classes/MediaLibraryTest.php +++ b/tests/unit/system/classes/MediaLibraryTest.php @@ -4,6 +4,12 @@ use System\Classes\MediaLibrary; class MediaLibraryTest extends TestCase // @codingStandardsIgnoreLine { + protected function tearDown() + { + $this->removeMedia(); + parent::tearDown(); + } + public function invalidPathsProvider() { return [ @@ -62,4 +68,63 @@ class MediaLibraryTest extends TestCase // @codingStandardsIgnoreLine $result = MediaLibrary::validatePath($path); $this->assertInternalType('string', $result); } + + public function testListFolderContents() + { + $this->setUpStorage(); + $this->copyMedia(); + + $contents = MediaLibrary::instance()->listFolderContents(); + $this->assertNotEmpty($contents, 'Media library item is not discovered'); + + $item = reset($contents); + $this->assertAttributeEquals('file', 'type', $item, 'Media library item does not have the right type'); + $this->assertAttributeEquals('/text.txt', 'path', $item, 'Media library item does not have the right path'); + $this->assertAttributeNotEmpty('lastModified', $item, 'Media library item last modified is empty'); + $this->assertAttributeNotEmpty('size', $item, 'Media library item size is empty'); + } + + protected function setUpStorage() + { + $this->app->useStoragePath(base_path('storage/temp')); + + config(['filesystems.disks.test_local' => [ + 'driver' => 'local', + 'root' => storage_path('app'), + ]]); + + config(['cms.storage.media' => [ + 'disk' => 'test_local', + 'folder' => 'media', + 'path' => '/storage/app/media', + ]]); + } + + protected function copyMedia() + { + $mediaPath = storage_path('app/media'); + + if (!is_dir($mediaPath)) { + mkdir($mediaPath, 0777, true); + } + + foreach (glob(base_path('tests/fixtures/media/*')) as $file) { + $path = pathinfo($file); + copy($file, $mediaPath . DIRECTORY_SEPARATOR . $path['basename']); + } + } + + protected function removeMedia() + { + if ($this->app->storagePath() !== base_path('storage/temp')) { + return; + } + + foreach (glob(storage_path('app/media/*')) as $file) { + unlink($file); + } + + rmdir(storage_path('app/media')); + rmdir(storage_path('app')); + } }