Bug #51562

Task #50876: Handling of deleted files in FAL

sys_file records marked as deleted crash file list (AKA: remove sys_file.deleted flag)

Added by Francois Suter almost 6 years ago. Updated 11 months ago.

Status:
Closed
Priority:
Should have
Category:
File Abstraction Layer (FAL)
Target version:
Start date:
2013-08-30
Due date:
% Done:

100%

TYPO3 Version:
6.1
PHP Version:
5.3
Tags:
Complexity:
Is Regression:
No
Sprint Focus:

Description

If some sys_file records have their "deleted" flag set to 1, the File > Filelist module will crash with exception "No file found for given UID". It seems like the deleted flag is not used anymore, but was obviously used at some point in the past, since I have a few records with such a status in my database. I guess I'm not the only one.

Reproducing the error condition is easy: access the sys_file table in some DB management tool, set the deleted flag of some file to 1 and try to access the folder where it is stored in the File module. This results in an exception.

I'm not sure how this is best corrected. I have seen that there's an intention to drop the deleted flag entirely, which seems fine if it is not used anyway, but that would be for 6.2 only. Something must be done for 6.0 and 6.1.


Related issues

Related to TYPO3 Core - Bug #52245: Revert BackendUtility::BEenableFields in FAL Rejected 2013-09-25
Blocks TYPO3 Core - Bug #51929: Creation of sys_file-table fails due to incorrect key Closed 2013-09-12

Associated revisions

Revision fcc250f5 (diff)
Added by Ernesto Baschny almost 6 years ago

[TASK] Remove sys_file.deleted flag and it's usage

This also fixes the 'Uncaught TYPO3 Exception #1317178604
No file found for given UID.' that appears when some sys_file
entries have a deleted flag set by mistake.

Resolves: #51562
Releases: 6.2, 6.1, 6.0
Change-Id: Id23636d2732f3562b8a155025656b26041c9a4e2
Reviewed-on: https://review.typo3.org/23567
Reviewed-by: Frans Saris
Tested-by: Frans Saris
Reviewed-by: Wouter Wolters
Reviewed-by: Stefan Neufeind
Tested-by: Stefan Neufeind

Revision cc7bde67 (diff)
Added by Ernesto Baschny almost 6 years ago

[TASK] Remove sys_file.deleted flag and it's usage

This also fixes the 'Uncaught TYPO3 Exception #1317178604
No file found for given UID.' that appears when some sys_file
entries have a deleted flag set by mistake.

Resolves: #51562
Releases: 6.2, 6.1, 6.0
Change-Id: Id23636d2732f3562b8a155025656b26041c9a4e2
Reviewed-on: https://review.typo3.org/23742
Reviewed-by: Stefan Neufeind
Tested-by: Stefan Neufeind

Revision 5db38d38 (diff)
Added by Ernesto Baschny almost 6 years ago

[TASK] Remove sys_file.deleted flag and it's usage

This also fixes the 'Uncaught TYPO3 Exception #1317178604
No file found for given UID.' that appears when some sys_file
entries have a deleted flag set by mistake.

Resolves: #51562
Releases: 6.2, 6.1, 6.0
Change-Id: Id23636d2732f3562b8a155025656b26041c9a4e2
Reviewed-on: https://review.typo3.org/23748
Reviewed-by: Stefan Neufeind
Tested-by: Stefan Neufeind

Revision 6ebc13a4 (diff)
Added by Anja Leichsenring almost 6 years ago

[BUGFIX] Move forgotten getEnvironmentMode()

getWhereClauseForEnabledFields() moved from StorageRepository
to AbstractRepository, but getEnvironmentMode() stayed
in StorageRepository although getWhereClauseForEnabledFields()
relies on it. Move getEnvironmentMode() as well.

Change-Id: Id252c8bd3b9e09a2c38d5ea1ebe6497dd76c12ae
Resolves: #51562
Releases: 6.2, 6.1, 6.0
Reviewed-on: https://review.typo3.org/23759
Reviewed-by: Helmut Hummel
Tested-by: Helmut Hummel

Revision 322bbef7 (diff)
Added by Anja Leichsenring almost 6 years ago

[BUGFIX] Move forgotten getEnvironmentMode()

getWhereClauseForEnabledFields() moved from StorageRepository
to AbstractRepository, but getEnvironmentMode() stayed
in StorageRepository although getWhereClauseForEnabledFields()
relies on it. Move getEnvironmentMode() as well.

Change-Id: Id252c8bd3b9e09a2c38d5ea1ebe6497dd76c12ae
Resolves: #51562
Releases: 6.2, 6.1, 6.0
Reviewed-on: https://review.typo3.org/23760
Reviewed-by: Helmut Hummel
Tested-by: Helmut Hummel

Revision 1831c954 (diff)
Added by Anja Leichsenring almost 6 years ago

[BUGFIX] Move forgotten getEnvironmentMode()

getWhereClauseForEnabledFields() moved from StorageRepository
to AbstractRepository, but getEnvironmentMode() stayed
in StorageRepository although getWhereClauseForEnabledFields()
relies on it. Move getEnvironmentMode() as well.

Change-Id: Id252c8bd3b9e09a2c38d5ea1ebe6497dd76c12ae
Resolves: #51562
Releases: 6.2, 6.1, 6.0
Reviewed-on: https://review.typo3.org/23761
Reviewed-by: Helmut Hummel
Tested-by: Helmut Hummel

History

#1 Updated by Steffen Ritter almost 6 years ago

all access to the deleted flag in FAL needs to be removed, as well as it's definition in TCA ctrl section and for 6.2 even the sql field itself

This has been defined on the FAL Sprint in Mainz - it just did not happen yet. The missing file handling has been done before.

The patch should be straight forward.

History:
In the original FAL the deleted flag of the table was not part of the concept - the propertiy in the object actually has the same name than the database field which lead to the mistake that it was persisted or peope thought it was the same. By thinking fixing it, people started taking care of the flag which lead to errors and now an undefined state

#2 Updated by Alexander Opitz almost 6 years ago

Please can you post a backtrace and which Typo3 6.1 version do you use.

#3 Updated by Alexander Opitz almost 6 years ago

@Steffen

Will this change backported to 6.0/6.1? As it may break extensions.

#4 Updated by Francois Suter almost 6 years ago

Alexander Opitz wrote:

Please can you post a backtrace and which Typo3 6.1 version do you use.

Version is 6.1.3 (the same happens with 6.1-dev). Here's the backtrace:

Uncaught TYPO3 Exception
#1317178604: No file found for given UID. (More information)

TYPO3\CMS\Core\Resource\Exception\FileDoesNotExistException thrown in file
/usr/local/src/typo3/TYPO3_6-1/typo3/sysext/core/Classes/Resource/ResourceFactory.php in line 281.

11 TYPO3\CMS\Core\Resource\ResourceFactory::getFileObject(139)

/usr/local/src/typo3/TYPO3_6-1/typo3/sysext/core/Classes/Resource/ProcessedFileRepository.php:
00094:   */
00095:  protected function createDomainObject(array $databaseRow) {
00096:   $originalFile = $this->resourceFactory->getFileObject(intval($databaseRow['original']));
00097:   $originalFile->setStorage($this->resourceFactory->getStorageObject($originalFile->getProperty('storage')));
00098:   $taskType = $databaseRow['task_type'];

10 TYPO3\CMS\Core\Resource\ProcessedFileRepository::createDomainObject(array)

/usr/local/src/typo3/TYPO3_6-1/typo3/sysext/core/Classes/Resource/ProcessedFileRepository.php:
00159: 
00160:   if (is_array($databaseRow)) {
00161:    $processedFile = $this->createDomainObject($databaseRow);
00162:   } else {
00163:    $processedFile = $this->createNewProcessedFileObject($file, $taskType, $configuration);

9 TYPO3\CMS\Core\Resource\ProcessedFileRepository::findOneByOriginalFileAndTaskTypeAndConfiguration(TYPO3\CMS\Core\Resource\File, "Image.Preview", array)

/usr/local/src/typo3/TYPO3_6-1/typo3/sysext/core/Classes/Resource/Service/FileProcessingService.php:
00091:   $processedFileRepository = Utility\GeneralUtility::makeInstance('TYPO3\\CMS\\Core\\Resource\\ProcessedFileRepository');
00092: 
00093:   $processedFile = $processedFileRepository->findOneByOriginalFileAndTaskTypeAndConfiguration($fileObject, $taskType, $configuration);
00094: 
00095:   // set the storage of the processed file

8 TYPO3\CMS\Core\Resource\Service\FileProcessingService::processFile(TYPO3\CMS\Core\Resource\File, TYPO3\CMS\Core\Resource\ResourceStorage, "Image.Preview", array)

/usr/local/src/typo3/TYPO3_6-1/typo3/sysext/core/Classes/Resource/ResourceStorage.php:
00788:    throw new \InvalidArgumentException('Cannot process files of foreign storage', 1353401835);
00789:   }
00790:   $processedFile = $this->getFileProcessingService()->processFile($fileObject, $this, $context, $configuration);
00791: 
00792:   return $processedFile;

7 TYPO3\CMS\Core\Resource\ResourceStorage::processFile(TYPO3\CMS\Core\Resource\File, "Image.Preview", array)

/usr/local/src/typo3/TYPO3_6-1/typo3/sysext/core/Classes/Resource/File.php:
00291:   */
00292:  public function process($taskType, array $configuration) {
00293:   return $this->getStorage()->processFile($this, $taskType, $configuration);
00294:  }
00295: 

6 TYPO3\CMS\Core\Resource\File::process("Image.Preview", array)

/usr/local/src/typo3/TYPO3_6-1/typo3/sysext/filelist/Classes/FileList.php:
00620:        // Thumbnails?
00621:        if ($this->thumbs && $this->isImage($ext)) {
00622:         $processedFile = $fileObject->process(\TYPO3\CMS\Core\Resource\ProcessedFile::CONTEXT_IMAGEPREVIEW, array());
00623:         if ($processedFile) {
00624:          $thumbUrl = $processedFile->getPublicUrl(TRUE);

5 TYPO3\CMS\Filelist\FileList::formatFileList(array, "file")

/usr/local/src/typo3/TYPO3_6-1/typo3/sysext/filelist/Classes/FileList.php:
00351:    }
00352:    // Files are added
00353:    $iOut .= $this->formatFileList($files, $titleCol);
00354:    // Header line is drawn
00355:    $theData = array();

4 TYPO3\CMS\Filelist\FileList::getTable("fileext,tstamp,size,rw,_REF_")

/usr/local/src/typo3/TYPO3_6-1/typo3/sysext/filelist/Classes/FileList.php:
00197:   */
00198:  public function generateList() {
00199:   $this->HTMLcode .= $this->getTable('fileext,tstamp,size,rw,_REF_');
00200:  }
00201: 

3 TYPO3\CMS\Filelist\FileList::generateList()

/usr/local/src/typo3/TYPO3_6-1/typo3/sysext/filelist/Classes/Controller/FileListController.php:
00288:    $this->filelist->start($this->folderObject, $this->pointer, $this->MOD_SETTINGS['sort'], $this->MOD_SETTINGS['reverse'], $this->MOD_SETTINGS['clipBoard'], $this->MOD_SETTINGS['bigControlPanel']);
00289:    // Generate the list
00290:    $this->filelist->generateList();
00291:    // Write the footer
00292:    $this->filelist->writeBottom();

2 TYPO3\CMS\Filelist\Controller\FileListController::main()

/usr/local/src/typo3/TYPO3_6-1/typo3/sysext/filelist/mod1/index.php:
00044: $SOBE = \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance('TYPO3\\CMS\\Filelist\\Controller\\FileListController');
00045: $SOBE->init();
00046: $SOBE->main();
00047: $SOBE->printContent();
00048: ?>

1 require("/usr/local/src/typo3/TYPO3_6-1/typo3/sysext/filelist/mod1/index.php")

/usr/local/src/typo3/TYPO3_6-1/typo3/mod.php:
00039:  require $temp_path . 'conf.php';
00040:  $BACK_PATH = '';
00041:  require $temp_path . 'index.php';
00042:  $isDispatched = TRUE;
00043: } else {

#5 Updated by Ernesto Baschny almost 6 years ago

  • Status changed from New to Accepted

#48336 introduced (mis)using the "deleted" flag. It was included in 6.0.7 and 6.1.2.

This was later reverted in 6.0.8 and 6.1.3.

So if you had one of 6.0.7 or 6.1.2 installed for some time you might have gotten "deleted" flag being set.

I propose that we remove the flag that was introduced as a misconception and stick to the rules set up in task #50876 which is what we decided in FAL Code Sprint in Mainz. So we should backport the removal of this flag also in 6.1 and 6.0. I don't expect "extensions to break". If yes, they have to adapt, because it's not worth really to support two separate "ways" of dealing with it.

#6 Updated by Gerrit Code Review almost 6 years ago

  • Status changed from Accepted to Under Review

Patch set 1 for branch master has been pushed to the review server.
It is available at https://review.typo3.org/23567

#7 Updated by Ernesto Baschny almost 6 years ago

  • Parent task set to #50876

#8 Updated by Ernesto Baschny almost 6 years ago

  • Subject changed from sys_file records marked as deleted crash file list to sys_file records marked as deleted crash file list (AKA: remove sys_file.deleted flag)

#9 Updated by Ernesto Baschny almost 6 years ago

  • Assignee set to Ernesto Baschny
  • Is Regression set to No

#10 Updated by Gerrit Code Review almost 6 years ago

Patch set 2 for branch master has been pushed to the review server.
It is available at https://review.typo3.org/23567

#11 Updated by Gerrit Code Review almost 6 years ago

Patch set 3 for branch master has been pushed to the review server.
It is available at https://review.typo3.org/23567

#12 Updated by Stefan Neufeind almost 6 years ago

!!! still needs a follow-up for 6.2 to remove the field completely

#13 Updated by Stefan Neufeind almost 6 years ago

removing that field was included in the patch - will take that into account for the backports then

#14 Updated by Gerrit Code Review almost 6 years ago

Patch set 1 for branch TYPO3_6-1 has been pushed to the review server.
It is available at https://review.typo3.org/23742

#15 Updated by Ernesto Baschny almost 6 years ago

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

#16 Updated by Gerrit Code Review almost 6 years ago

  • Status changed from Resolved to Under Review

Patch set 2 for branch TYPO3_6-1 has been pushed to the review server.
It is available at https://review.typo3.org/23742

#17 Updated by Gerrit Code Review almost 6 years ago

Patch set 1 for branch TYPO3_6-0 has been pushed to the review server.
It is available at https://review.typo3.org/23748

#18 Updated by Ernesto Baschny almost 6 years ago

  • Status changed from Under Review to Resolved

#19 Updated by Gerrit Code Review almost 6 years ago

  • Status changed from Resolved to Under Review

Patch set 1 for branch TYPO3_6-0 has been pushed to the review server.
It is available at https://review.typo3.org/23758

#20 Updated by Gerrit Code Review almost 6 years ago

Patch set 1 for branch master has been pushed to the review server.
It is available at https://review.typo3.org/23759

#21 Updated by Gerrit Code Review almost 6 years ago

Patch set 2 for branch master has been pushed to the review server.
It is available at https://review.typo3.org/23759

#22 Updated by Gerrit Code Review almost 6 years ago

Patch set 1 for branch TYPO3_6-1 has been pushed to the review server.
It is available at https://review.typo3.org/23760

#23 Updated by Gerrit Code Review almost 6 years ago

Patch set 1 for branch TYPO3_6-0 has been pushed to the review server.
It is available at https://review.typo3.org/23761

#24 Updated by Anja Leichsenring almost 6 years ago

  • Status changed from Under Review to Resolved

#25 Updated by Benni Mack 11 months ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF