Problem/Motivation

Currently, the minimum age to delete temporary files is 6 hours (configurable at /admin/config/media/file-system).
Also, it's possible to NEVER delete temporary files.

These are the current options:
Delete files

In some situations, for example when doing a file cleanup by setting files temporary, you wish to delete them "Immediately (on next Cron run)". Waiting to 6 hours or settings the value in config, isn't cool and I don't see a reason why that shouldn't be provided as option?

Steps to reproduce

Go to /admin/config/media/file-system
and try to set "Delete temporary files after" to immediately. The option doesn't exist.

Proposed resolution

Add an option to delete the files "Immediately (on next Cron run)"

As "never" has a value of "0", we should use "1" for this.

Additionally I'd like to propose adding the unit (seconds?) to the documentation, currently this doesn't seem documented?
https://git.drupalcode.org/search?search=temporary_maximum_age&nav_sourc...

Remaining tasks

  1. Discuss
  2. Implement
  3. Test
  4. Release

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
delete-temporary-files.png28.22 KBanybody

Issue fork drupal-3305127

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

Anybody created an issue. See original summary.

anybody’s picture

Issue summary: View changes
anybody’s picture

Issue summary: View changes
// Interval values in seconds:
    $intervals = [0, 1, 21600, 43200, 86400, 604800, 2419200, 7776000];
    $period = array_combine($intervals, array_map([$this->dateFormatter, 'formatInterval'], $intervals));
    $period[0] = $this->t('Never');
    $form['temporary_maximum_age'] = [
      '#type' => 'select',
      '#title' => $this->t('Delete temporary files after'),
      '#default_value' => $config->get('temporary_maximum_age'),
      '#options' => $period,
      '#description' => $this->t('Temporary files are not referenced, but are in the file system and therefore may show up in administrative lists. <strong>Warning:</strong> If enabled, temporary files will be permanently deleted and may not be recoverable.'),
    ];

Unsure if it's worth overwriting the options label from date formatter here for this special case instead of displaying "1 Second"?

longwave’s picture

Doesn't adding this option allow a race condition, where a temporary file is uploaded, then cron happens to run in the background before it can be processed and converted to a permanent file? This might cause bugs that are hard to track down or reproduce. I think that is why the previous minimum was 6 hours, to avoid this situation.

anybody’s picture

Thanks for the quick reaction @longwave. Puh, if that's the case, that's a mean trap. Perhaps that information should also be added as comment in the code?

I didn't know about this and indeed that makes it hard to implement, while I described a case where this functionality would be quite useful.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

mstrelan’s picture

Had the same thought as @longwave in #5. I think this can be closed? Or feel free to repurpose to update documentation as per #6.

shaikhshadab00’s picture

Assigned: Unassigned » shaikhshadab00
Issue tags: +Drupal Mumbai Meetup
catch’s picture

Status: Active » Postponed (maintainer needs more info)

#5 is the reason and iirc it matches the form cache lifetime. Moving to 'needs more info'. All I can think of is making the option same as form cache lifetime and then making that configurable, but that seems too complicated to present in a UI.

smustgrave’s picture

@anybody wonder if this should be closed out?

anybody’s picture

Assigned: shaikhshadab00 » Unassigned
Status: Postponed (maintainer needs more info) » Closed (won't fix)

Yes let's close it won't fix. It's just not worth it, while I think the use case is still there, but also edgy enough to close... Any if anyone will need it for good reasons in the future, we can reopen it and find a solution.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.