TYPO3 Core

This is just a (quick) suggestion:

diff --git a/typo3/sysext/core/Classes/Resource/Service/ExtractorService.php b/typo3/sysext/core/Classes/Resource/Service/ExtractorService.php
index 92229d3581..a4a05a75dc 100644
--- a/typo3/sysext/core/Classes/Resource/Service/ExtractorService.php
+++ b/typo3/sysext/core/Classes/Resource/Service/ExtractorService.php
@@ -80,6 +80,16 @@ class ExtractorService
      */
     private function isFileTypeSupportedByExtractor(File $file, ExtractorInterface $extractor): bool
     {
+        if ($file->getMetaData()->count() > 0) {
+            // There's a design flaw in FAL, moving a file should not reindex it
+            // because it will effectively lead to overriding any user-defined
+            // value by metadata extracted by any extractor again.
+            // We will do our small job to refuse to extract metadata if metadata
+            // are already present.
+            // See: https://forge.typo3.org/issues/91168
+            return false;
+        }
+
         $isSupported = true;
         $fileTypeRestrictions = $extractor->getFileTypeRestrictions();
         if (!empty($fileTypeRestrictions) && !in_array($file->getType(), $fileTypeRestrictions, true)) {

Something that could logically be moved even more near the actual call (currently trying to find a way to prevent this flaw in my ext until TYPO3 is fixed).

Hi Xavier, I am testing your extension and have noticed this issue (again).

Would you consider creating a patch for your suggested change?

I think metadata extraction on moving files does not make sense at all.

(I have not checked in later versions if this will still happen, maybe we should check that first).

In the extension, this is now implemented like this, comparing tstamp and crdate in sys_file_metadata:

public function canProcess(File $file)
    {
        $typo3Branch = class_exists(\TYPO3\CMS\Core\Information\Typo3Version::class)
            ? (new \TYPO3\CMS\Core\Information\Typo3Version())->getBranch()
            : TYPO3_branch;
        if (version_compare($typo3Branch, '10.0', '<')) {
            $metadata = $file->_getMetaData();
        } else {
            $metadata = $file->getMetaData();
        }

        if (!empty($metadata) && $metadata['crdate'] !== $metadata['tstamp']) {
            // There's a design flaw in FAL, moving a file should not reindex it
            // because it will effectively lead to overriding any user-defined
            // value by metadata extracted by any extractor again.
            // We will do our small job to refuse to extract metadata if metadata
            // modification timestamp is different from creation timestamp
            // See: https://forge.typo3.org/issues/91168
            return false;
        }

link to latest commit in master branch

I am not sure this is a design flaw. I think what happened is that probably at the beginning the metadata was in sys_file too and then this was separated and the extractors added and the file index (sys_file) and metadata (sys_file_metadata) has not been entirely and cleanly separated.

This is visible in the core and documentation.

I think to fix is good not to fix or workaround this one case but decide when sys_file should be updated and metadata extracted and separate these 2 from each other where it makes sense. Also, it is not so easy to fix and I suggest to write tests first to make sure not to break anything.

...

Clearly separated:

• 2 separate scheduler tasks
• automatic metadata extraction can be turned off in storage

Not clearly separated:

• in core there is a function Indexer::updateIndexEntry, this updates the sys_file index and starts metdata extraction. In some cases it is good to do both (e.g. when a new file is uploaded or a file copied), in some cases it does not make sense to do metadata extraction (again), e.g. on file move, but you do need the sys_file index updating.

public function updateIndexEntry(File $fileObject): File
    {
        $updatedInformation = $this->gatherFileInformationArray($fileObject->getIdentifier());
        $fileObject->updateProperties($updatedInformation);

        $this->getFileIndexRepository()->update($fileObject);
        $metaData = $this->extractRequiredMetaData($fileObject);

        if ($this->storage->autoExtractMetadataEnabled()) {
            $metaData = array_merge($metaData, $this->getExtractorService()->extractMetaData($fileObject));
        }
        $fileObject->getMetaData()->add($metaData)->save();
        return $fileObject;
    }

documentation:

• in the documentation, in some places the distinction between the file index (sys_file) and metadata extraction is not clear

e.g.

Indexing a file creates a database record for the file, containing meta-information both about the file (file-system properties) and from the file (e.g. EXIF information for images).

https://docs.typo3.org/m/typo3/reference-coreapi/12.4/en-us/ApiOverview/Fal/Architecture/Components.html#the-file-index

That is kind of correct if with "indexing" you mean updating sys_file and extracting metadata and with "database record" you mean sys_file and sys_file_metadata. In any case, I find it misleading.

Otherwise, it is not correct, because the exif metadata is written to sys_file_metdata

• also, the metadata extraction is not documented at all (e.g. how to add your own extractors)

I stumbled on this issue due to EXT:extractor not reindexing the files in case they are being "replaced" in the backend. I filed the issue here: https://github.com/xperseguers/t3ext-extractor/issues/88