Feature #70012: addFile should not delete the original file - TYPO3 Core - TYPO3 Forge

Actions

Copy link

Feature #70012

closed

addFile should not delete the original file

Added by Franz Holzinger about 9 years ago. Updated about 7 years ago.

Status:

Closed

Priority:

Could have

Assignee:

Category:

File Abstraction Layer (FAL)

Target version:

6.2.16

Start date:

2015-09-22

Due date:

% Done:

100%

Estimated time:

PHP Version:

Tags:

Complexity:

Sprint Focus:

Description

The ResourceStorage class has a method addFile from where it calls the driver classes addFile method. However the parameter $removeOriginal is not provided and passed.

public function addFile($localFilePath, Folder $targetFolder, $targetFileName = '', $conflictMode = 'changeName') {

It should be like this:

public function addFile($localFilePath, Folder $targetFolder, $targetFileName = '', $conflictMode = 'changeName', $removeOriginal = TRUE) {

interface DriverInterface:

Adds a file from the local server hard disk to a given path in TYPO3s
virtual file system. This assumes that the local file exists, so no
further check is done here! After a successful the original file must
not exist anymore. *
@param string $localFilePath (within PATH_site)
@param string $targetFolderIdentifier
@param string $newFileName optional, if not given original name is used
@param boolean $removeOriginal if set the original file will be removed
after successful operation
@return string the identifier of the new file

public function addFile($localFilePath, $targetFolderIdentifier, $newFileName = '', $removeOriginal = TRUE);

Reason:
During the development it is very helpful if the FAL added files are not removed from the computer harddisk. This can be done in a later stage.

Actions

Copy link

Updated by Christian Kuhn over 8 years ago

+1, just stumbled upon that, too.

My scenario: I have a file within my extension and I want to add it to the default storage within fileadmin/. Codewise, that is pretty simple:

        /** @var StorageRepository $storageRepository */
        $storageRepository = GeneralUtility::makeInstance(StorageRepository::class);
        $storage = $storageRepository->findByUid(1);
        $sourceLocation = GeneralUtility::getFileAbsFileName('EXT:styleguide/Resources/Public/Images/Pictures/bus_lane.jpg');
        $folder = $storage->getRootLevelFolder();
        try {
            $folder->createFolder('styleguide');
        } catch (ExistingTargetFolderException $e) {
            // No op if folder exists
        }
        $folder = $folder->getSubfolder('styleguide');
        $fileObject = $storage->addFile($sourceLocation, $folder, 'bus_lane');

But addFile always removes the original file afterwards! Woot? This is bad for multiple reasons:

Wrong naming: addFile() always moves, so it should be called moveFile(), but moveFile() exists already, but can not be used to add a local file, it needs a file object for a storage, not a local file.

Wrong encapsulation: Deciding on the deletion of the original file is NOT task of the add method. Only a surrounding controller can know the fate of the original.

To mitigate the issue, I'd +1 Franz request to at least add an additional argument to addFile to suppress the originial file deletion.

Actions

Copy link

Updated by Christian Kuhn over 8 years ago

Side question: Why does the DriverInterface addFile() have the argument $removeOriginal in the first place? The target driver knows too much about the source this way. I shouldn't take care of the original file at all.

Actions

Copy link

Updated by Christian Kuhn over 8 years ago

Do I miss something? If not, why did this not pop up earlier? That's sort of a basic operation, isn't it?

Actions

Copy link

Updated by Gerrit Code Review over 8 years ago

Status changed from New to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/47383

Actions

Copy link

Updated by Gerrit Code Review over 8 years ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/47383

Actions

Copy link

Updated by Gerrit Code Review over 8 years ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/47383

Actions

Copy link

Updated by Gerrit Code Review over 8 years ago

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/47383

Actions

Copy link

Updated by Gerrit Code Review over 8 years ago

Patch set 1 for branch TYPO3_7-6 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/47422

Actions

Copy link