Provide a way to keep the original timestamp when using file_save(), use it in file classification batch

[indigoxela]

Description of the need

This started as a conversation on Zulip.

When upgrading from D7 the File entity type has to get determined, which happens in function file_type_determine().
That simply uses file_save(), which does a lot more and among other things it sets the type. And it updates the timestamp.

File entities only have one timestamp property, which gets set to REQUEST_TIME in FileStorageController::preSave().
It always gets set, so there's no way to keep the timestamp, even if just updating some metadata, not the file itself.

This leads to the situation, that after upgrading from Drupal or whenever doing some batch logic with File entities, they get the REQUEST_TIME set. And after that it's no longer possible to sort files by (their actual) age - that's the bug part here.

Proposed solution

A new property to prevent updating the timestamp if requested.

Alternatives that have been considered

Hackish stuff to fix the all-files-same-age situation by scripting.

Additional information

Nothing so far...

Draft of feature description for Press Release (1 paragraph at most)

Backdrop now provides a way to keep the original timestamp of File entities when saving.

[indigoxela]

So here's my idea: backdrop/backdrop#5305

To quickly test with devel:

$f = file_load(1);

$f->keep_timestamp = TRUE;

file_save($f);

And then check on admin/content/files whether the timestamp's still the same.

Feedback welcome!

[argiepiano]

Thanks. The PR is very targeted and does the job. I would suggest changing the title of the issue to state the fact that, you are not only providing a way to keep the original stamp, but you are also adding code so that the type reclassification batch doesn't alter the timestamp of the file.

Looking at the PR, this is hard-coded; in other words, running the reclassifying batch will always keep the original timestamp. I think this is fine - I can't think of any situation when core or contrib might want to run that batch (or call file_type_determine()) and not keep the original timestamp.

At first I wondered if it might just be better to avoid the new property, and simply skip updating the timestamp if the file is not new... but this property provides maximum flexibility, and doesn't alter core's behavior - thus it's BC. So, core will still keep updating timestamp when you do a "normal" file_save() without specifying the property.

As a sidenote - we will need to alter the view headers at admin/content/files so that it's not "Uploaded" but "Last saved" or something like that.

A minor nitpik: can we use camelCase or the new property? I know a lot of older classes don't do that, but newer core classes do.

[indigoxela]

... this property provides maximum flexibility, and doesn't alter core's behavior - thus it's BC

Yes, that's the idea behind it: make it fully opt-in, no changes to default behaviors.

For Backdrop 2.x we might consider adding a "changed" property to actually differentiate between "created" and "updated" - like in other Entity classes. But that can't happen in a minor release (IMO).

... we will need to alter the view headers at admin/content/files so that it's not "Uploaded" but "Last saved" or something...

I "smell" a wording discussion. 😁 But agreed, this table header is quite misleading. I'm just not sure if this should happen here - it's rather unrelated to file_save(). Does opening a new issue for that make sense to you @argiepiano?

A minor nitpik: can we use camelCase

Yes, of course. Done.

[indigoxela]

Functional tests added.

[argiepiano]

Functional tests added.

Great! Still LGTM!

Does opening a new issue for that make sense

Done. #7174