Project

General

Profile

Actions

Task #67126

closed

Bug #59473: Inconsistent API for folders

Bug #65305: Folder::getSubfolder() doesn't use the file drivers correctly

Make sure ProcessedFile and LocalImageProcessor use correct FAL API

Added by Frans Saris almost 9 years ago. Updated over 5 years ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
-
Target version:
-
Start date:
2015-05-25
Due date:
% Done:

100%

Estimated time:
TYPO3 Version:
7
PHP Version:
Tags:
Complexity:
medium
Sprint Focus:

Description

ProcessedFile::setName() and LocalImageProcessor::checkForExistingTargetFile() only work for local/'normal' file systems where the file identifier is the full filepath.

As not all file systems use this to build the identifiers of files and folders these functions should use the proper FAL methods to make sure it works in all cases.


Related issues 2 (0 open2 closed)

Related to TYPO3 Core - Bug #67095: unnecessary generation of imagesClosedFrans Saris2015-05-21

Actions
Related to TYPO3 Core - Bug #75756: Fatal error occurs when sys_file_processedfile table is cleared but processed file is still presentClosed2016-04-18

Actions
Actions #1

Updated by Markus Klein almost 9 years ago

Maybe more places have some special treatment for "Local" driver type. Don't forget to grep them and check all.

Actions #2

Updated by Michael Oehlhof almost 9 years ago

I have had a look in the sources and here are my results:

In LocalImageProcessor.php we need the absolute path to the target file. But there is no public function in the driver interface yet. Because we need it only for the ‚Local’ DriverType we can change in the LocalDriver.php the function getAbsolutePath from protected to public and use this function.
To use the function getAbsolutePath we also must get the current driver in the function checkForExistingTargetFile, for this we need to change the function getDriver of ResourceStorage.php from protected to public.

In the function setName of ProcessedFile.php the property ‚identifier‘ is set directly. That did not work with drivers that are creating the identifiers themselves in a special way. We have to use the function addFile of the storage to add the file and then use the identifier of the added file.

Any comments welcome.

Actions #3

Updated by Frans Saris almost 9 years ago

The LocalImageProcessor can also be used by non Local storages. So only adding this to the local driver isn't enough.

Actions #4

Updated by Michael Oehlhof almost 9 years ago

In LocalImageProcessor the "new" function is only called for the local storage, so adding this to the local driver is enough.
Line 106: if (.... && $storage->getDriverType() === 'Local')

Actions #5

Updated by Frans Saris almost 9 years ago

Had a look at the code and now I get your intention.

But IMO we should find a solution that fits all type of drivers. The $storage->getDriverType() === 'Local' check is added because this works only for hierarchical path based storages and there is currently no way to check for this other then checking for type == local

Actions #6

Updated by Michael Oehlhof almost 9 years ago

For the processed files we need direct access to the file which is than used by getImageDimensions.
Do we need a solution for drivers which filesystems are not locally accessible?

Actions #7

Updated by Philipp Wrann over 8 years ago

We are currently working on a FAL Driver implementation for our own media database and ran into that problem, processedFile generation (backend preview for example) is not possible because of conditions like:

if ($processingFolder->hasFile($task->getTargetFileName()) && $storage->getDriverType() === 'Local')

How are the chances for this to be fixed in 7 LTS?

Actions #8

Updated by Daniel Hölbling-Inzko over 8 years ago

I just ran into this issue with a custom FAL Driver I was writing. Unfortunately this severely limits the usability of the FAL in our case as we where going to connect Typo3 to an external Image-Repository through it. And having reprocessed files on every request slows the system down to a crawl.

Anything I can do to help to fix this issue?
Looking through the code it seems the FAL Driver and Storage is missing some methods to retrieve local filenames for processing am I right? Because most code from `checkForExistingTargetFile` should imo move into storage/driver.

greetings Daniel

Actions #9

Updated by Martin Kleofasz over 8 years ago

@Daniel Hinderink Hölbling-Inzko

Have you work out a solution for this problem?
We encountered the same problem with our custom FAL driver and also with the EXT:fal_ftp from the extension repository, if the storage is flagged readonly.

If we can help to fix this issue. Contact me please.

Actions #10

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/46905

Actions #11

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/46905

Actions #12

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/46905

Actions #13

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/47127

Actions #14

Updated by Anonymous about 8 years ago

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

Updated by Benni Mack over 5 years ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF