Steps to reproduce:

1. Create a new entity with a media file.
2. Create a revision of that entity and change the media file.
3. Delete the entity.
4. Attempt to delete the file added in step 1 - this will fail.

The reason is that when the entity is deleted media_file_delete() is invoked by the field api to remove references in the file_usage table. The bug is that it only decrements the file in the current revision. It really should be able to delete all references to that entity, not just that file.

I attempted a few different fixes before settling on this patch. IMO, what really should happen is that the field api should invoke hook_field_delete_revision() for each revision, but the field attach api really doesn't have any concept of the revision data.

Anyway, this is a simple fix that bypasses the field_usage_delete() function and deletes straight from the database. It's a little unclean, but works well without any side effects in my tests.

CommentFileSizeAuthor
media.fields.inc_.file_usage_delete.patch997 byteskevin hankens

Comments

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

i don't think this would work if you've got the same image attached to a node in two different fields, it would delete the usage for both. The proper solution here would be to allow media/%file/delete to work even though a file has usage, just like we fixed in 2.x of file_entity.

kevin hankens’s picture

kevin hankens commented

Haha, I love the comment for file_delete_multiple :)

/**
 * Delete multiple files.
 *
 * Unlike core's file_delete(), this function does not care about file usage
 * or skip on invalid URIs. Just deletes the damn file like it should.
 ...

Although, if I'm not mistaken, the patch above should do the same thing that file_usage_delete() will do if you pass it $count = 0. It just does it in a heavy-handed way. If the same file was referenced by two different fields, it should be handled the same way because of the $count = 0 - the first pass will delete the usage of the second.

That said, the media admin page is probably a better authority of when to delete. There is a bunch of code in there that lists where the file is being used. Perhaps if file_usage isn't empty, but it can't load any referencing entities, we just delete the sucker at that point.

kevin hankens’s picture

kevin hankens commented

Oh yeah, I missed your point about file being referenced by two fields. I was only looking at deleting an entity - nott removing a single field. That would probably break something. I'll try to roll a patch with a more forceful delete from the media listing page.

chris matthews’s picture

chris matthews commented
Issue summary: View changes
Status: Active » Closed (outdated)

Closing this issue as outdated. However, if you think this issue is still important, please let us know and we will gladly re-open it for review.
sincerely,
- the Drupal Media Team