Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Updated: Comment #N
This might be a duplicate of an existing issue. Unfortunately the issue search seems to be broken atm and I can't check that.
Problem/Motivation
Currently hook_entity_delete() is sometimes called after the data already were deleted. This leads to situations in which it's impossible to do the necessary clean-up tasks because those tasks would rely on the already deleted data.
So far I know only of taxonomy and comments that invoke those deletion hooks after deleting the data. The node, user and file module already invoke the hook before deleting.
Proposed resolution
Change the order of hook invocation / data deletion in the modules taxonomy and comments.
Remaining tasks
Reviews needed.
User interface changes
none
API changes
Not really an API change, but it could change the available data in the affected hooks.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | drupal-invoke-deletion-hooks-before-deleting-data-2180647-2.patch | 2.88 KB | das-peter |
| drupal-invoke-deletion-hooks-before-deleting-data.patch | 2.61 KB | das-peter | |
Comments
Comment #2
das-peter CreditAttribution: das-peter commentedChanged the code in the comments module to call
_comment_update_node_statistics()after the data are removed. Locally the previously failing tests pass now.Comment #3
das-peter CreditAttribution: das-peter commentedThis was fixed for D8 in #1346032: Ordering of hook_entity_delete() is inconsistent
Comment #4
xjmSo, hm. This is a bit of an API change, and it makes me a little nervous to consider it for D7. Let's check with David to get an idea whether this would be acceptable for D7 or not.
Also, if it can be considered for D7, it will need automated test coverage (see the D8 issue for how test coverage was added there) but let's get David's feedback first.
Comment #5
David_Rothstein CreditAttribution: David_Rothstein commentedHm, yeah, that inconsistency is unfortunate but also looks a little dangerous.
Given that the entity is passed to the hook, it seems like most code wouldn't care whether it's in the database or not anymore (although I guess some could). Are there good examples of where this is needed?
Maybe we could consider introducing a separate "pre-delete" hook instead?
Comment #6
das-peter CreditAttribution: das-peter commented@David_Rothstein Well I've stumbled over this issue while fixing the issue I've set as related: #2176317: Delete term related physical folder
Maybe we could consider introducing a separate "pre-delete" hook instead?Absolutely, I think that would be a viable solution. All I need is a "delete" hook that is invoked while the data is still fully available.
Comment #7
nickonom CreditAttribution: nickonom commentedI am hitting the same issue as described on https://drupal.stackexchange.com/questions/254629
Comment #8
nickonom CreditAttribution: nickonom commentedI've found another issue, which discusses introducing pre-delete hook, so I am closing this one and marking it as duplicate.
Comment #9
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedThat issue is for the Entity module, though, so it won't help with the core entities (such as taxonomy terms and comments) discussed in this issue, right? Drupal core would still need to introduce and use that hook.
Comment #10
nickonom CreditAttribution: nickonom commentedI see the difference now, David. Good point!