Orphan Purger not triggered when deleting revisions

A test would be useful to demonstrate this, but looks sensible and simple enough.

Created an MR based on Remco Hoeneveld patch to allowing to check if a entity orphan up to the root of the composite hierarchy.

Our use case: a multi-level hierarchy of paragraphs, some paragraphs containing other paragraphs.

We added some custom presave entity hooks calling the new _entity_reference_revisions_delete_entity_orphans with the new $recursive_usage_check to be able to cleanup all the potential orphaned entities.

Updated patch from MR to fix small typo in function name: refrence -> reference.

Instead of a a prefixed "internal" function, what if we already convert this to the new #Hook + #LegacyHook hook syntax? A single method can then implement both hooks on 11.1+ and the two old hooks can be marked as #LegacyHook. That's fully forward compatible and will not need to be touched anymore until D10 support is dropped.

https://www.drupal.org/node/3442349 →

Tried something with MR 62.

Nice, reviewed.

Thanks for the quick turnaround. Tests would be nice, but I'm OK with merging this as it is now.

berdir → closed merge request !60

Automatically closed - issue fixed for 2 weeks with no activity.

Note: \Drupal\entity_reference_revisions\Plugin\Field\FieldType\EntityReferenceRevisionsItem::deleteRevision() already exists, so deleting a revision should in most cases directly delete that revision, so this will add considerably extra load on deleting revisions as it will expand the queue :-/

Reopening this for now, considering to revert this because this is a considerable overhead when doing everything twice. I'd like to understand the scenario when this is needed better. One option would be to remove revisionDelete().

Hi @berdir, what do you mean exactly by "doing everything twice"?

I mean that we'd immediately delete the revision but then also register a queue item to check whether or not we should delete that revision.

So we need to figure out when and why deleteRevision() doesn't do its job, a test for that would be very helpful.

The reason I commented and re-opened is that I noticed today on a client project that a node had 5000 revisions and node_revision_delete was somehow not running properly. So I did run the manual batch, and then, knowing about this issue, did run the manual ERR orphan purger. But it barely did anything and upon closer inspection I realized that basically all the revisions already had been deleted. So it was working as it should, for me anyway.

Our use case is to be able to properly delete a specific revision of a node, not the whole node, without having to run the orphan purger.

The problem might be that be that we add several times the same entity to the orphan purger queue, possibly many times if we have a lot of revisions and we delete the node entirely.

Could it be possible to store in a request cache the entities already added to the orphan purger queue so we don't add them several times?

Ok, I think I see what you mean when you say that everything is done twice.

When running the orphan purger batch we don't need to add the processed entities to the purger queue.

Is there a way to know that the hook are being called from the batch processing?

If so, we could just skip the processing in such a situation.

Anyway, I tried to work on a static cache to avoid processing multiple times the same entity in the same request, MR added to this issue.