[BUGFIX] Folder::getFiles directly calls Factory::createFileObject 60/26360/2
authorSteffen Ritter <info@rs-websystems.de>
Thu, 12 Dec 2013 18:50:26 +0000 (19:50 +0100)
committerSteffen Ritter <info@rs-websystems.de>
Sat, 21 Dec 2013 10:03:36 +0000 (11:03 +0100)
Folder::getFiles implements the logic of creating file objects
itself, after retrieving the information from the driver.
Besides the fact that this is slow since all information for the
object are received from the filesystem directly instead of the
cache in the sys_file table the uid is not present in these
objects which finally results in the lack of metadata in these
file objects.

In addition to that ommiting the ResourceFactory several objects
for the same file might exists which may lead to inconsistent
behaviour and output on modifying the file.

As the Folder/File Objects only should be a convinience facade
in front of the ResourceStorage this change introduces a new
method their, implementing the new and improved logic.

At the same time the old functionality - which enforces manual
file object creation - has been deprecated and the filelist
module is adapted accordingly.

Releases: 6.2
Resolves: #53688
Change-Id: I3fb97d432d325bd6400c0ae208b90d702c9f528d
Reviewed-on: https://review.typo3.org/26360
Reviewed-by: Frans Saris
Tested-by: Frans Saris
Reviewed-by: Steffen Ritter
Tested-by: Steffen Ritter
typo3/sysext/core/Classes/Resource/Folder.php
typo3/sysext/core/Classes/Resource/ResourceStorage.php
typo3/sysext/core/Tests/Unit/Resource/FolderTest.php
typo3/sysext/filelist/Classes/FileList.php

index 1ff7f6d..0d1b9cb 100644 (file)
@@ -193,20 +193,7 @@ class Folder implements FolderInterface {
                        list($backedUpFilters, $useFilters) = $this->prepareFiltersInStorage($filterMode);
                }
 
-               /** @var $factory ResourceFactory */
-               $factory = \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance('TYPO3\\CMS\\Core\\Resource\\ResourceFactory');
-               $fileArray = $this->storage->getFileList($this->identifier, $start, $numberOfItems, $useFilters, TRUE, $recursive);
-               $fileObjects = array();
-               foreach ($fileArray as $fileInfo) {
-                       $fileObject = $factory->createFileObject($fileInfo, $this->storage);
-
-                       // we might have duplicate filenames when fetching a recursive list, so don't use the filename as array key
-                       if ($recursive == TRUE) {
-                               $fileObjects[] = $fileObject;
-                       } else {
-                               $fileObjects[$fileInfo['name']] = $fileObject;
-                       }
-               }
+               $fileObjects = $this->storage->getFilesInFolder($this, $start, $numberOfItems, $useFilters, $recursive);
 
                $this->restoreBackedUpFiltersInStorage($backedUpFilters);
 
@@ -223,8 +210,7 @@ class Folder implements FolderInterface {
         * @return integer
         */
        public function getFileCount(array $filterMethods = array(), $recursive = FALSE) {
-               // TODO replace by call to count()
-               return count($this->storage->getFileList($this->identifier, 0, 0, $filterMethods, FALSE, $recursive));
+               return count($this->storage->getFileIdentifiersInFolder($this->identifier, TRUE, $recursive));
        }
 
        /**
index 4f84ba6..5ab0334 100644 (file)
@@ -1320,10 +1320,10 @@ class ResourceStorage {
         * @param boolean $loadIndexRecords If set to TRUE, the index records for all files are loaded from the database. This can greatly improve performance of this method, especially with a lot of files.
         * @param boolean $recursive
         * @return array Information about the files found.
+        * @deprecated since 6.2, will be removed two versions later
         */
-       // TODO check if we should use a folder object instead of $path
-       // TODO add unit test for $loadIndexRecords
        public function getFileList($path, $start = 0, $numberOfItems = 0, $useFilters = TRUE, $loadIndexRecords = TRUE, $recursive = FALSE) {
+               GeneralUtility::logDeprecatedFunction();
                // This also checks for read permissions on folder
                $folder = $this->getFolder($path);
                $rows = array();
@@ -1341,6 +1341,44 @@ class ResourceStorage {
                return $items;
        }
 
+       /**
+        * @param Folder $folder
+        * @param int $start
+        * @param int $maxNumberOfItems
+        * @param bool $useFilters
+        * @param bool $recursive
+        * @return File[]
+        */
+       public function getFilesInFolder(Folder $folder, $start = 0, $maxNumberOfItems = 0, $useFilters = TRUE, $recursive = FALSE) {
+               $this->assureFolderReadPermission($folder);
+
+               $rows = $this->getFileIndexRepository()->findByFolder($folder);
+
+               $filters = $useFilters == TRUE ? $this->fileAndFolderNameFilters : array();
+               $fileIdentifiers = $this->driver->getFileIdentifierListInFolder($folder->getIdentifier(), $recursive, $filters);
+               $fileIdentifiersCount = count($fileIdentifiers);
+               $items = array();
+               if ($maxNumberOfItems === 0) {
+                       $maxNumberOfItems = $fileIdentifiersCount;
+               }
+               $end = min($fileIdentifiersCount, $start + $maxNumberOfItems);
+               for ($i = $start; $i < $end; $i++) {
+                       $identifier = $fileIdentifiers[$i];
+                       if (isset($rows[$identifier])) {
+                               $fileObject = $this->getFileFactory()->getFileObject($rows[$identifier]['uid'], $rows[$identifier]);
+                       } else {
+                               $fileObject = $this->getFileFactory()->getFileObjectByStorageAndIdentifier($this->getUid(), $identifier);
+                       }
+                       $key = $fileObject->getName();
+                       while (isset($items[$key])) {
+                               $key .= 'z';
+                       }
+                       $items[$key] = $fileObject;
+               }
+               uksort($items, 'strnatcasecmp');
+
+               return $items;
+       }
 
        /**
         * @param string $folderIdentifier
index 790f02c..cdce9a0 100644 (file)
@@ -96,7 +96,7 @@ class FolderTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
         */
        public function getFilesReturnsArrayWithFilenamesAsKeys() {
                $mockedStorage = $this->getMock('TYPO3\\CMS\\Core\\Resource\\ResourceStorage', array(), array(), '', FALSE);
-               $mockedStorage->expects($this->once())->method('getFileList')->will($this->returnValue(array(
+               $mockedStorage->expects($this->once())->method('getFilesInFolder')->will($this->returnValue(array(
                                'somefile.png' => array(
                                        'name' => 'somefile.png'
                                ),
@@ -119,8 +119,8 @@ class FolderTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
                $mockedStorage = $this->getMock('TYPO3\\CMS\\Core\\Resource\\ResourceStorage', array(), array(), '', FALSE);
                $mockedStorage
                        ->expects($this->once())
-                       ->method('getFileList')
-                       ->with($this->anything(), $this->anything(), $this->anything(), $this->anything(), $this->anything(), FALSE)
+                       ->method('getFilesInFolder')
+                       ->with($this->anything(), $this->anything(), $this->anything(), $this->anything(), FALSE)
                        ->will($this->returnValue(array()));
 
                $fixture = $this->createFolderFixture('/somePath', 'someName', $mockedStorage);
@@ -134,8 +134,8 @@ class FolderTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
                $mockedStorage = $this->getMock('TYPO3\\CMS\\Core\\Resource\\ResourceStorage', array(), array(), '', FALSE);
                $mockedStorage
                        ->expects($this->once())
-                       ->method('getFileList')
-                       ->with($this->anything(), $this->anything(), $this->anything(), $this->anything(), $this->anything(), TRUE)
+                       ->method('getFilesInFolder')
+                       ->with($this->anything(), $this->anything(), $this->anything(), $this->anything(), TRUE)
                        ->will($this->returnValue(array()));
 
                $fixture = $this->createFolderFixture('/somePath', 'someName', $mockedStorage);
index 24442f1..0ae1f92 100644 (file)
@@ -282,15 +282,16 @@ class FileList extends \TYPO3\CMS\Backend\RecordList\AbstractRecordList {
                // TODO use folder methods directly when they support filters
                $storage = $this->folderObject->getStorage();
                $storage->resetFileAndFolderNameFiltersToDefault();
-               $folders = $storage->getFolderList($this->folderObject->getIdentifier());
-               $files = $storage->getFileList($this->folderObject->getIdentifier());
+
                // Only render the contents of a browsable storage
+
                if ($this->folderObject->getStorage()->isBrowsable()) {
+                       $folders = $storage->getFolderList($this->folderObject->getIdentifier());
+                       $files = $this->folderObject->getFiles();
                        $this->sort = trim($this->sort);
                        if ($this->sort !== '') {
                                $filesToSort = array();
-                               foreach ($files as $file) {
-                                       $fileObject = $storage->getFile($file['identifier']);
+                               foreach ($files as $fileObject) {
                                        switch ($this->sort) {
                                                case 'size':
                                                        $sortingKey = $fileObject->getSize();