Problem/Motivation

Recently @catch, @webchick, @xjm, @cottser, @cilefen and I discussed #2801777: Give users the option to prevent drupal from automatically marking unused files as temporary , #2810355: $entity->isDefaultTranslation() behaves incorrectly when changing default translation, causing file/image field usage to be set to zero, causing files to be deleted and #2708411: [PP-1] editor.module's editor_file_reference filter not tracking file usage correctly for translations. We all agreed that the current file usage behaviour that is resulting in files being deleted when they are being used by a site is unacceptable and a critical issue.

This was also discussed at Drupal Dev Days Seville — see the meeting notes.

This issue is exist to propose a series of steps to mitigate and fix these related issues.

The current behaviour

As files are "used" in different situations we add to the counter in the file_usage table. As the usages are removed we subtract from the counter. Once the counter gets to 0 the file is made temporary in \Drupal\file\FileUsage\FileUsageBase::delete(). Then a cron process (file_cron()) comes along and deletes temporary files after system.file:temporary_maximum_age seconds.

There are several cases where that counter is not incremented or decremented correctly.

Regardless of all the sub-issues we have the known issue that this does not track usages added by content editors if they are able to add links to file but editing the source HTML. Also the system of maintaining a counter seems very fragile and evidence supports that :).

Which cases/issues are there?

These fall into clear buckets, along three axes:

  1. Field (image/file/entity reference) versus editor.module's editor_file_reference filter
  2. Entity revisions versus entity translations
  3. Data loss: yes or no
Short description Data loss? Field or filter? Revision or translation? Issue Notes
Deleting entity with revisions + image/file field no Field Revision #1239558: Deleting an entity with revisions and file/image field does not release file usage of non-default revisions, causing files to linger No data loss, but irrepairable, therefore still critical.
Changing default translation + image/file field YES Field Translation #2810355: $entity->isDefaultTranslation() behaves incorrectly when changing default translation, causing file/image field usage to be set to zero, causing files to be deleted This was caused by the "fix" for another critical: #2787187: Data loss: Deleting a translation of an entity deletes all file_usage entries for files referenced by the entity
The critical #2666700: User profile images unexpectedly deleted was merged into this — it has the same root cause, but different steps to reproduce.
Generic entity reference fields YES Field N/A #2826127: Usage of Files Referenced in Entity Reference Fields Not Tracked A missing feature. File usage support is only present on file and image fields, which subclass the generic entity reference field code.
Filter + translations YES Filter Translation #2708411: [PP-1] editor.module's editor_file_reference filter not tracking file usage correctly for translations Was implemented using entity hooks before Translation API matured.
Filter + revisions + adding/removing images no Filter Revision #2892368: editor.module's hook_entity_update() does not count file usages correctly when adding/removing images without creating new revisions The only "normal" priority issue here, because it cannot lead to data loss, and can be repaired
The mitigation for all the rows with YES in the Data loss? column N/A #2801777: Give users the option to prevent drupal from automatically marking unused files as temporary This is a mitigation for:
  • existing sites that desperately need a solution NOW
  • sites where it doesn't matter if files end up getting deleted or not (running out of disk space is only an issue on sites where there is a lot of editing that includes the need for replacing one file with another, without having an old revision still pointing to it)

The downside: it becomes difficult to delete "unused" files, which can be a security problem (e.g. accidentally uploaded PDF with sensitive data, now how to delete it?). That's less of a problem than files just disappearing right under you though. Worst case, the site administrator could be asked to remove the file from disk manually.

Proposed resolution

TBD

Remaining tasks

TBD

User interface changes

TBD

API changes

TBD

Data model changes

TBD

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

alexpott created an issue. See original summary.

catch’s picture

catch’s picture

Issue tags: +Triaged D8 critical

This was opened as a result of a triage discussion, adding the tag to reflect that.

alexpott’s picture

Issue summary: View changes
Leksat’s picture

My personal thoughts on this.

1. #2801777: Give users the option to prevent drupal from automatically marking unused files as temporary would be cool only for some projects. Disk space will run out pretty fast on a project having tens of editors, thousands of files, and frequent content updates.

2. > does not track usages added by content editors if they are able to add links to file but editting the source HTML
But we can add a support for this, right? The editor module already parses formatted text fields. It could search for img tags too.

3. > the system of maintaining a counter seems very fragile
We just need to fix few bugs and extend tests to make it strong again.

In general, is this issue mean that the file usage system can be removed completely? I'm asking because I don't see alternatives.
Also file usage is really helpful sometimes when checking issues reported by clients.

Leksat’s picture

Also, IMHO it would be nice if "Delete orphaned files after" would be "1 day" by default (it's "6 hours" currently) because most of hosting providers make backups on daily basis.

Berdir’s picture

1:
> on a project having tens of editors, thousands of files, and frequent content updates.

Frequent content updates that replace images? I don't think that is a very common use case, usually sites continuously create new content and don't delete/update existing one? At least not on a massive scale?

2:
Editor module parses image tages for a UUID, trying to do that for arbitrary image links is going to be a lot harder, imagine direct linkes to image styles, external images (both actually external images and those that point to your own domain), what about embedded videos, links to PDf's and so on.. that's *a lot* of markup/reference variants that we'd have to cover.

Also keep in mind that we are moving towards having media entity in core. That means each file is actually part of a media entity, which means the file itself is always "used". We could add a similar tracking for any entity usage or specifically for media entities but that currently doesn't exist yet in the existing module. So those media entities will be part of the re-usable media library, and just because you stop using one for a while, that doesn't mean you want to have it removed from the library. So even if we would add usage tracking, we wouldn't automatically delete I think.

One thing that would be possible is to let users view files/medias without usage and delete them explicitly if they are no longer needed. Of course we also need to make the usage tracking system as reliable as possible. And sites that do use this can then still ensure that they only use supported embedding mechanisms, while others can still do whatever they want with their content/markup.

catch’s picture

3. > the system of maintaining a counter seems very fragile
We just need to fix few bugs and extend tests to make it strong again.

This misses that as soon as anything goes wrong, we have no way currently to rebuild the counts.

Leksat’s picture

@catch,
> we have no way currently to rebuild the counts.
Can't understand this. In #2708411: [PP-1] editor.module's editor_file_reference filter not tracking file usage correctly for translations we have a patch with an update hook that updates the file usage values to correct numbers.
Or do you mean something else?

catch’s picture

That fixes it for one case, it doesn't rebuild it for all cases, and there's no API for this it's a one-off update.

saurabh.tripathi.cs’s picture

Hi Everyone,

####Update after some debugging####

Drupal Version used: 8.2.1
Lingotek module used:
Machine name: lingotek
Version: 8.x-1.12


1) Uploaded one image in existing content in a node's slice (read more about slices here ) via WYSIWYG editor, saved the node and than checked below tables:
1.1) file usage
1.2) file managed
Result: The usage count in file_usage table is set as 2, but it should be 1, as i am sure that we are using that image only in one node object.

2) Created a new content, added a new slice
added a new image to it, and saved the node.
Result: file usage count shows 8 instead of 1.

3) Applied patch 2708411-44.patch provided here and ran hook update.
now on adding new image in existing or new content
Result: file usage entry is not created for image.

Drupal Version used: 8.2.4 and 8.2.1
Lingotek module not used:

1)The upload behavior works fine, on uploading 1 image, i entry created in file_managed and usage count shows 1 in file_usage table,
2) On uploading same image in same node, 2nd entry created in file managed, usage count shows 2 but an extra record is created in file usage of same file with usage count 1. (is that normal?)
explanation: If we are not removing previous image and uploading same image again, than drupal should consider it as a new image, and it does also by creating a new fid in file managed table, but,
the usage count for the removed file should not be affected, instead it gets incremented by 1.
A new record for the new image is created in file_usage table with count 1, however see point 3 below.

3) There is one bug: If we edit the content and change any field or not and click on save, the file usage count in file_usage table increases by one against the file which is uploaded in that node.
Also the second record which is of previous image (because we uploaded the same image again in the node and not removed previous one.) the usage count increments by one.


  • Summary: Usage count increments by one on editing and saving the content only. This is because a new revision is created every time we edit and save the node, how ever if we remove a file from node (via WYSIWIG editor) than the usage count is not decremented.
  • With lingotek module also enabled, the usage count jumps directly to 8 even if we upload a single image in a new node.

#####How we checked ########
Copy the uuid of image from node edit page and search it in file_managed table.
Copy the fid of record and search it in file_usage table, count field displays the no of times the file is being used. (in my case value comes as 2 for existing content and 8 for new content)
Copy the id field value: This is the id of entity object which is using the file. This object can be a node, slice etc.

######Questions######

1) Will running the db_update provided here fix this?
2) How does garbage collection of drupal 8 works?
2.1) When a file has usage count as 0, does that file gets deleted along with their entry in file managed and usage tables?
2.2) Which condition is checked first: file_managed has entry or file_usage count is 0 before deleting a file.
2.3) What triggers the increment of file usage count, what code/file?

####Work-Around??###
If we don't get a fix on this soon we are planning to set up a separate cron which will run a script checking all files which are not in file managed table and are in file system and remove them, also the one which have file usage count 0 will be removed.

There i can add extra condition to crosscheck if a file in file managed table and recount object using this file.

We will un-check the check-box of default garbage collection of drupal in that case, i am not sure if that workaround is a good idea.

Please suggest how to proceed,
Thanks.

hitfactory’s picture

This bug is causing a lot of frustration for content editors on one of our multilingual sites.

I think the fundamental problem is that core makes the assumption that files not associated with entities are no longer required and should be deleted.

In our case, only administrators and content editors can upload files to the server to begin with. And our client expects that when they upload a file to the website (either via a field or via CKEditor etc), that the file will remain on the server regardless of any changes to the host entity. This is because they often link to a file or image on the website from email campaigns.

Can someone explain what the security issue is in regards to allowing unused files to remain on the server?

Berdir’s picture

There are two different versions of unused files. Those that were never used in the first place, e.g. because they were uploaded in (possibly public) forms and then never submitted or replaced before they were ever marked as used. You don't want to keep those, which is why completely disabling deleting temporary files is IMHO a bad idea.

The other case of files that were once used and no longer are is a very different scenario. The only problem there is that your disk could eventually get full if you have a site that replaces a lot of files and no longer uses old ones. That's what #2801777: Give users the option to prevent drupal from automatically marking unused files as temporary is about, it got stuck on the admin UI and texts there. I'd love some help there to move that forward.

And as also mentioned, media entity will completely change this as well, because a file in a media entity will always be used by itself, so media entities won't be deleted automatically, ever.

Leksat’s picture

Berdir’s picture

There is also #2708411: [PP-1] editor.module's editor_file_reference filter not tracking file usage correctly for translations for embedded images and then there's the whole problem of manually added links (for example to uploaded PDF files) and HTML in content that we are not tracking at all :)

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

catch’s picture

So I still think we need to stop using file usage to programmatically delete files at all. We could potentially retain it (at least in 8.x) as a usage estimate on the file admin pages. Theoretical cases like sites that regularly change files or delete lots of content are nothing compared to the actual cases of files getting deleted on real sites.

Leksat’s picture

There is a blog post from mail.ru - one of the most popular email services in Russia.
They use a file usage system similar to Drupal's one, but with additional protection for dealing with unexpected file deletions.
https://www.smashingmagazine.com/2017/01/reducing-email-storage-mailru/#...

Yet, they never delete all file usages at once. Each usage added or removed individually.
(I'm talking about the ability to call \Drupal\file\FileUsage\FileUsageInterface::delete() with $count == 0 which deletes all file usages.)

TrevorBradley’s picture

I just hit this independently and posted about block inline images going missing over in #2863829: Block inline images considered "orphaned", deleted?.

Is there any way to see which files are flagged for deletion, or put the system into a debug mode that says "I would have deleted this file if file deletion were turned on for reason X"?

Wim Leers’s picture

Issue summary: View changes

This was also discussed at Drupal Dev Days Seville — see the meeting notes.

@xjm asked me to get all of the files criticals going again. So I did. I did deep dives on every issue. I posted summaries of all discussion on each of the issues, updated their title, issue summaries, and so on.

This enabled me to get the overview across this cluster of about half a dozen critical issues. Turns out the entire space of issues… falls into clear buckets, along three axes:

  1. Field (image/file/entity reference) versus editor.module's editor_file_reference filter
  2. Entity revisions versus entity translations
  3. Data loss: yes or no

From this, I distilled a table that hopefully makes it very easy to find your way through these issues:

Short description Data loss? Priority Field or filter? Revision or translation? Issue Notes
Deleting entity with revisions + image/file field no Critical Field Revision #1239558: Deleting an entity with revisions and file/image field does not release file usage of non-default revisions, causing files to linger No data loss, but irrepairable, therefore still critical.
Changing default translation + image/file field YES Critical Field Translation #2810355: $entity->isDefaultTranslation() behaves incorrectly when changing default translation, causing file/image field usage to be set to zero, causing files to be deleted This was caused by the "fix" for another critical: #2787187: Data loss: Deleting a translation of an entity deletes all file_usage entries for files referenced by the entity
The critical #2666700: User profile images unexpectedly deleted was merged into this — it has the same root cause, but different steps to reproduce.
Generic entity reference fields YES Critical Field N/A #2826127: Usage of Files Referenced in Entity Reference Fields Not Tracked A missing feature. File usage support is only present on file and image fields, which subclass the generic entity reference field code.
Filter + translations YES Critical Filter Translation #2708411: [PP-1] editor.module's editor_file_reference filter not tracking file usage correctly for translations Was implemented using entity hooks before Translation API matured.
Filter + revisions + adding/removing images no Normal Filter Revision #2892368: editor.module's hook_entity_update() does not count file usages correctly when adding/removing images without creating new revisions The only "normal" priority issue here, because it cannot lead to data loss, and can be repaired
The mitigation for all the rows with YES in the Data loss? column N/A #2801777: Give users the option to prevent drupal from automatically marking unused files as temporary This is a mitigation for:
  • existing sites that desperately need a solution NOW
  • sites where it doesn't matter if files end up getting deleted or not (running out of disk space is only an issue on sites where there is a lot of editing that includes the need for replacing one file with another, without having an old revision still pointing to it)

The downside: it becomes difficult to delete "unused" files, which can be a security problem (e.g. accidentally uploaded PDF with sensitive data, now how to delete it?). That's less of a problem than files just disappearing right under you though. Worst case, the site administrator could be asked to remove the file from disk manually.

(This table is now also in the IS, but I'm also posting it in this comment so it can be easily compared later on, to track the evolution.)

Wim Leers’s picture

Regardless of all the sub-issues we have the known issue that this does not track usages added by content editors if they are able to add links to file but editing the source HTML.

We could easily make \Drupal\editor\Plugin\Filter\EditorFileReference reject any <img> tag that doesn't have the data-entity-type and data-entity-uuid attributes set, or if their values don't result in a successful entity_load(). That would prevent this. It'd also disallow external linking, but this could be a setting.

That fixes it for one case, it doesn't rebuild it for all cases, and there's no API for this it's a one-off update.

Can you give an example of which use case is absolutely impossible to recalculate?

Related: https://www.drupal.org/project/auditfiles might be helpful for some of the sites running into this problem space.

Wim Leers’s picture

Issue summary: View changes

I forgot that d.o's node preview was so terrible. Removed the Priority column so that more of the table is legible.

plach’s picture

Awesome work, Wim, that helps a lot! :)

Wim Leers’s picture

d.o's CSS still gets in the way, here's a screenshot with some HTML + CSS hackery to make the table more useful:

@plach: <3

Eli-T’s picture

@wim-leers the auditfiles module you linked to in #21 has no D8 version, at least findable from its d.o project page.

catch’s picture

Can you give an example of which use case is absolutely impossible to recalculate?

I mean it's impossible to recalculate the file_usage table as a whole, it relies on the owner of a specific implementation identifying and fixing a bug, then writing the code to do that in a one-off update. There's no way as a site-owner to run that code/debug it etc except possibly extracting it from an update (if that update even exists).

So I still feel like there are fundamental issues with the file_usage system that means we should focus efforts on a replacement rather than trying to fix it:

- the increment/decrement doesn't give you any idea where files are used, if you wanted to use that information to audit.
- any bug in file usage tracking can result in data-loss at any arbitrary later date (unless it's fixed and a correct upgrade path is written). Since the same file can be referenced in multiple different ways, a single module with a bad implementation can put any file on the site at risk.
- an upgrade path that attempts to rebuild file usage data will have to collect the data at the time it runs - that could could potentially be used by a different API that pulls the usages rather than using increment/decrement

That leads towards doing something like this:

1. Remove the removal of zero-usage files altogether
2. Deprecate the file_usage API
3. Allow manual deletion of files from the UI, show file_usage data as an interim indication.
4. Build a new API that allows you to obtain counts and individual usages of individual files on-demand (via references, text fields etc.) - obviously some of this information is more expensive to get than others. This could then be used by sites that want to audit for unused files.

So if we're able to rebuild the information for every instance of file usage, then we should be able to use that for an on-demand usage audit API that's more useful to people. If we're not, then we can't fix what we have anyway, and have to revert to something informational anyway.

ckaotik’s picture

4. Build a new API that allows you to obtain counts and individual usages of individual files on-demand (via references, text fields etc.) - obviously some of this information is more expensive to get than others. This could then be used by sites that want to audit for unused files.

We've recently built a (currently private) module for a client that collects information of entity references via various means (CKEditor entity_embed tokens, entity_reference fields, blocks in panels, ...). This might be of interest?
What we have noticed there though, is that it's really hard to get information for all kinds of references. For example, how to deal with custom forms, config entities etc. that don't use the Field API for storage? I think some kind of generic entity usage tracking API would be wonderful for this, so both core and contrib could integrate with it and we wouldn't need to keep different tracking solutions for different types/sources of references.

Wim Leers’s picture

I mean it's impossible to recalculate the file_usage table as a whole, it relies on the owner of a specific implementation identifying and fixing a bug, then writing the code to do that in a one-off update.

True. But that's achievable, at least for core. There aren't boatloads of things outside core dealing with files.

the increment/decrement doesn't give you any idea where files are used, if you wanted to use that information to audit.

Not true — /admin/content/files.

  1. Allow manual deletion of files from the UI, show file_usage data as an interim indication.

I'm curious to hear what Berdir thinks about this.

  1. Build a new API that allows you to obtain counts and individual usages of individual files on-demand (via references, text fields etc.) - obviously some of this information is more expensive to get than others. This could then be used by sites that want to audit for unused files.

I fear that on most sites this would be prohibitively expensive. Let alone on big sites. Let alone for files referenced via text fields.

catch’s picture

the increment/decrement doesn't give you any idea where files are used, if you wanted to use that information to audit.

Not true — /admin/content/files.

If I upload a file to a node, then I unpublish the node, how do I find out that the file is only referenced by one, unpublished, node at the moment?

I fear that on most sites this would be prohibitively expensive. Let alone on big sites. Let alone for files referenced via text fields.

Yes, so it might not be an option, but it'd still be better than deleting random files.

Also, we could look at 'files referenced by text fields' being paired to an administrative field on the entity, or tracking table (k/v or similar), so that it only has to be calculated on entity save.

dawehner’s picture

I would totally argue in favour of removing the file usage API, its complex and doesn't even the kind of things people do.
Not only that, but I also fear that the temporary feature itself is sort of broken:

Random example I debugged the other week. The client used the ckeditor file upload for pdfs, copied the link and then pasted it into a link field, which in their usecase seemed reasonable, as they also had external PDFs. Given that the file have never been tracked in the first place, as it was never part of a normal file upload field NOR the wysiwyg editor, still, the file was totally usable by the user, but then disappeared after a while. Maybe this is an edge case, but its something the client came up with for themselves.

jonathanshaw’s picture

Re #21/#25 The auditfiles module has no D8 version but file_checker does, and does some of the same things: it ensures that all file entities in Drupal have an actual file on server.

3. Allow manual deletion of files from the UI, show file_usage data as an interim indication.
4. Build a new API that allows you to obtain counts and individual usages of individual files on-demand (via references, text fields etc.) - obviously some of this information is more expensive to get than others. This could then be used by sites that want to audit for unused files.

how about this:
A: extend the current file usage API to tracks instances of actual use, at the time of use. If we record and delete usages individually, its less fragile than a simple integer increment/decrement. For that matter, the media library examples discussed here show there's a serious use case for a more generic entity use API - so that we can explore and consider deleting unused media.

B: create a second file usage API that interrogates all the places files (or entities?) are used and rebuilds the usages records.

C: allow users to run B from the file UI as an action for selected files.

D: run B automatically when a user wishes to delete a file from the UI. If a new previously unrecorded usage is discovered, warn the user on the deletion confirmation page. Have something similar for programmatic deletion of unused files (a "safeDelete" method that checks usage before deletion).

This way we can BOTH
1. Show people a view of unused files that has a greater chance of being right, and show it instantly without a hugely expensive calculation
2. Prevent people from inadvertently deleting files that have usage

Wim Leers’s picture

#2801777: Give users the option to prevent drupal from automatically marking unused files as temporary landed. Does that mean we downgrade all other "file usage" criticals listed in the IS from critical to major?


#29:

If I upload a file to a node, then I unpublish the node, how do I find out that the file is only referenced by one, unpublished, node at the moment?

Touché! Thanks for that insight, I hadn't realized that yet!
Well, actually … none of the file usage tracking is affected by whether a node is published or not? Published or not, file usage is tracked, and it shows up correctly at /admin/content/files.

#30: You can't upload PDFs in CKEditor unless with just core. You can if you install https://www.drupal.org/project/editor_file. It's then up to editor_entity_insert() & friends to detect the inserted file and mark it as permanent. If the file was uploaded, the URL copied, and then the HTML removed, then this hook will never be able to detect the inserted file and mark it as permanent. Your client was definitely using the system in a way it was not meant to be used. If they didn't remove the HTML, they must've run into one of the two "filter" rows in the IS.

catch’s picture

Well, actually … none of the file usage tracking is affected by whether a node is published or not? Published or not, file usage is tracked, and it shows up correctly at /admin/content/files.

This is exactly what I mean though.

The argument that when deleting a node, you expect the file to be deleted applies equally to unpublishing a node and expecting the file to be unpublished. In both cases, if the file stays on disk and accessible via the public file directory, it could be available when you don't expect it.

If we don't automatically delete files, but the file usage API is functioning perfectly otherwise, then you could look at a view of zero usage files, and delete the file manually when doing a file audit.

Where the file is attached to an unpublished node, it will show up with a usage of '1', but there's no way from the managed files page to know which node it's attached to and whether or not that node is published or not. We can change that situation though if we track usages one by one.

Wim Leers’s picture

I see, thanks for clarifying!

SKAUGHT’s picture

to add a thought (i can't find a more specific issue to address this toward)

assuming revision & workflow, referenced entities, and all the toys are counted correctly: perhaps at the point when a file is unused it is displayed under a new UI:
/admin/content/files/unused
to help, at least, visually filter this type of garbage collection.

as well toward other possible future forward tools (like a Trash mode) then files will still exist and can be reclaimed when un-trashed.

ckaotik’s picture

@skaught There is already a listing of all temporary files at /admin/content/files?status=0. Those are the files that will be deleted, if pruning is enabled.
The problem I see with reclaiming is that once the last file usage is removed, you cannot know where the file was referenced before. Keeping a "These are the files you recently orphaned" table would get huge very quickly. And even if you knew where the file was attached, the default upload widget does not allow to re-use an existing file.

For the suggestion of a way to delete unused files via UI, there is already a separate issue: #2648816: D8: uploaded files are impossible to replace.

Slightly off-topic: I don't know if this use case is already represented in #2821423-24: Dealing with unexpected file deletion due to incorrect file usage's table or if there is a separate issue already: When editing an existing entity with a file field and selecting "Remove", the file is immediately deleted from disk, even if the user later decides to abort the edit of the entity. I don't think this is related to file usage though, just wanted to mention it, as it seems like an unexpected file deletion, too.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

vaplas’s picture

Sorry, if this scenario has already been discussed (issues by unexpected file deletion a lot):

1. Install 8.5.x
2. Add to theme-settings.php additional field settings with image

function bartik_form_system_theme_settings_alter(&$form, &$form_state) {

  $form['super_image'] = array(
    '#type' => 'managed_file',
    '#title' => t('My super image.'),
    '#upload_location' => 'public://images/',
    '#default_value' => theme_get_setting('super_image'),
  );

}

3. upload image
4. check database tables:

  • file_usage -> none
  • file_managed -> status = 0

5. After 6 hours (by default), the file will be deleted.

All known to me сontrib themes use this way to storing files too. So the problem has quite a wide impact. If this has not been discussed yet, I can write tests for it. Also, as a solution, we can check the status of files inside calling theme_get_setting, and change it, if necessary.

Berdir’s picture

Whoever uploads a file *is* responsible for making a file permanent (and registering usages). This isn't a bug, this is how the API works. I'm sure it could be improved, e.g. a flag to autotomatically mark files as permanent but that would be a task/feature.

This is required to clean up files that are uploaded, e.g. on new nodes and then the node is never actually saved.

vaplas’s picture

@Berdir, thank you for help with it! You right, I overlooked the code with set permanent and add to file.usage via hook_submit(). And yes, simplify work with api is task/feature. I will create an issue for this, if I do not find an existing one (maybe #618654: File and image fields are treated as temporary files and automatically deleted after six hours).

But I also noticed, that even if the user uses the Bartik, and changes the default logo to custom logo (via UI, not api), then it will be stored only 6 hours too. It looks questionable behavior. I think I already saw an issue like "Why drupal removes my custom logo," but I just can not find it :(