Project

General

Profile

Actions

Feature #70012

closed

addFile should not delete the original file

Added by Franz Holzinger over 8 years ago. Updated over 6 years ago.

Status:
Closed
Priority:
Could have
Assignee:
-
Category:
File Abstraction Layer (FAL)
Target version:
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 #1

Updated by Christian Kuhn about 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 #2

Updated by Christian Kuhn about 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 #3

Updated by Christian Kuhn about 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 #4

Updated by Gerrit Code Review about 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 #5

Updated by Gerrit Code Review about 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 #6

Updated by Gerrit Code Review about 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 #7

Updated by Gerrit Code Review about 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 #8

Updated by Gerrit Code Review about 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 #9

Updated by Christian Kuhn about 8 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100
Actions #10

Updated by Riccardo De Contardi over 6 years ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF