Dbafs does not handle files that are being replaced

[Toflar]

I just digged into the Dbafs class once again and I found that when I do this

// foobar.jpg contains a 2000 x 2000 Pixel image
Dbafs::addResource('foobar.jpg');


// foobar.jpg now contains a 100 x 100 Pixel image
Dbafs::addResource('foobar.jpg');

the md5 hash is not updated. I found that you do that check here: https://github.com/contao/core/blob/develop/system/modules/core/drivers/DC_Folder.php#L920-L933

However, I think that's wrong because then everybody has to do that? It should be done in Dbafs::addResource(), right?

[leofeyer]

That's actually correct, because addResource() only adds the file if it does not yet exist.

[Toflar]

Hmm...still, that method should be centralized, no?

[fritzmg]

👍 I also think this should be done in \Dbafs::addResource()

[leofeyer]

As discussed on Mumble on October 14th, we should update the timestamp and file hash in the Dbafs class on line 100 (Return the model if it exists already).

[leofeyer]

I have been working on this one today and it turned out that it is not a good idea to always update the folder hash (as @ausi already mentioned). In the core, we are using Dbafs::addResource() 11 times, from which 6 times we do not need to update the hash and 5 times we do.

So there are two possible implementations:

// 1. Always update the folder hash but check for existence beforehand
$file = FilesModel::findByPath('…');

if (null === $file) {
    $file = Dbafs::addResource($file);
}

// do something with $file

// 2. We add another flag to the method
$file = Dbafs::addResource($file, true, true); // the second true means "update the hash"

I tend to number 1, because I think it is wrong to use Dbafs::addResource() to retrieve the model. We should use FilesModel::findByPath() instead.

@contao/developers Any objections against implementation no. 1?

[Toflar]

👍 for 1.

[leofeyer]

Changed in contao/core-bundle@dcb46f3.