Add events for matching entity hooks

Just a quick note: we shouldn't be adding this on the annotation. This is storage handler specific, which means that top-level annotation keys may become obsolete when the handler is changed. I know we've done similar things before, but it's confusing and bad practice.

I would probably argue for event-specific classes rather than entity type specific classes. I'd be happy to hear arguments for other cases, though.

Well yeah I think for a start we should go with ContentEntityEvent | ConfigEntityEvent and then see whether there are actually usecases for more specific ones.
I doubt we can apply domain specific events given the way how entity API works and how people interact with that.

One question I would have, and I think we should think about is whether we should have a dedicated event dispatcher for those events. Its a total different domain compared to many other ones out there already.

As I said on IRC, there's no point in event-specific classes when they all carry the same payload (the entity being acted upon).

I must have missed that message. I'd argue the update event could technically be extended with the original entity as it was known to its storage before it was updated.

I do in all honesty miss the point of having different events for content and config entities. I assume you have use cases for this?

There's no value-add to a new domain dispatcher unless they need to be namespaced separately. Symfony has one single domain dispatcher, plus one per-form in its form API. (Doctrine does its own thing, which is silly Doctrine stuff.) Let's keep it in the single dispatcher.

I'd say an event class per event makes the most sense, not per-entity type. Per-entity-type means you'd have to create a bunch of stub objects every time you define a new entity type, which seems like a waste. There's enough boilerplate to creating a new entity as is. We should have a separate class per event, though, so EntityPresaveEvent, EntityCreateEvent, EntityInsertEvent, etc. (Although the naming may need to be clearer than hooks; it's totally non-obvious which are pre-hooks and which are post-hooks in most cases. Let's name the events to be self-evident.)

For hooks, we have both entity-generic and type-specific hooks. Do we want to continue that for events? My knee-jerk response is yes for the reasons bojanz mentions in #6, but could be convinced otherwise.

Also, I don't see why this should vary by backend. Rather, these events should be fired consistently regardless of the storage engine. If I as a module developer can't rely on them being the same, then my use of a particular event locks me in to a given backend implementation implicitly. Let's not do that. If a storage driver doesn't implement events at the appropriate times, file a bug against the driver. No annotation needed.

Re event per class: I think about it semantically. What event just happened? An event was just deleted. That's the event, so that's the event class name. They mean different things, even if they are by default extremely similar. Think of it like section/article/aside tags vs. all divs. :-) Also, they may not be identical. $event->getOriginalEntity() would only exist on the update events, for instance. Even if they're super similar, and maybe share a trait or two for reducing boilerplate, they're still semantically distinct.

Re naming: Ideally, we would build events that do NOT need to get renamed in D9. Build what we want it to be, make it as good as possible, and that makes the transition easier. Ideally for Drupal 9 those events... don't change at all. We just remove the deprecated hooks. (That assumes we don't switch to a CRAP-only model in D9, which IMO we should, but at least those events that we keep wouldn't change names.) Better and more self-evident naming would, I'd argue, encourage event usage over hook usage, which I would consider a good thing.

Added #2553169: Dispatch bundle CRUD events as related.

Also, they may not be identical. $event->getOriginalEntity() would only exist on the update events, for instance. Even if they're super similar, and maybe share a trait or two for reducing boilerplate, they're still semantically distinct.

Agreed, the events are not 100% the same so modelling them as separate classes fits better and will saves us from semantic troubles with small difference like getOriginalEntity. Thus +1 on separate event classes.

Here are tests written against a potential implementation.

The test module had to be moved, it wasn't being picked up.
Additionally, the entity has to be saved before it can be deleted.

I added constants to EntityEvents for the main hooks we care about.
However, any subclass of EntityStorageBase could call invokeHook for any arbitrary hook, and we won't have an event for it.
ContentEntityStorageBase does this, for hooks like hook_entity_field_values_init, hook_entity_translation_create, etc.

I added a hardcoded array, otherwise we could have done constant('EntityEvents::' . strtoupper($hook));

Any ideas there?

CI error in this case is missing @group annotation in the test.

Aha! Did you figure that out from the test results somehow? Or just by looking at the patch. I couldn't see anything about @group in the console output.

1. +++ b/core/lib/Drupal/Core/Entity/EntityStorageBase.php
   @@ -163,6 +163,18 @@ protected function setStaticCache(array $entities) {
   +      \Drupal::service('event_dispatcher')->dispatch($hook_to_event_map[$hook], new EntityEvent($entity));
   

   This \Drupal::service() call is causing a bunch of unit tests to fail.

2. +++ b/core/modules/system/tests/modules/entity_test_event/entity_test_event.info.yml
   @@ -0,0 +1,3 @@
   +name: entity_test_event
   +type: module
   +core: 8.x
   

   Needs package: Testing

It's pretty much the goto reason for "DrupalCI is giving a failure, and I cant see anything in the logs to indicate why".

Right now, if the Error is immediately following the run-tests.sh --list, then it is almost certainly a missing @group tag:

00:00:26.880 php /var/www/html/core/scripts/run-tests.sh  --list --php /opt/phpenv/shims/php > /var/www/html/artifacts/testgroups.txt
00:00:26.881 Command created as exec id 7c7a86e9
00:00:27.516 HTTP/1.1 200 OK
00:00:27.517 Content-Type: application/vnd.docker.raw-stream
00:00:27.517 
00:00:27.517 
00:00:27.517 Error

We've got a plan to fix this so its not so obtuse: #2680713: Handle missing @groups

You rock tim, seriously!

1. +++ b/core/lib/Drupal/Core/Entity/EntityEvents.php
   @@ -0,0 +1,64 @@
   +  /**
   +   * @todo.
   +   *
   +   * @Event
   +   *
   +   * @var string
   +   */
   +  const PRESAVE = 'entity.presave';
   

   What about postsave?

2. +++ b/core/lib/Drupal/Core/Entity/EntityStorageBase.php
   @@ -163,6 +163,18 @@ protected function setStaticCache(array $entities) {
   +    if (isset($hook_to_event_map[$hook])) {
   +      \Drupal::service('event_dispatcher')->dispatch($hook_to_event_map[$hook], new EntityEvent($entity));
   +    }
   

   Should we invoke them now after or before?

1) hook_entity_update and hook_entity_insert are the equivalent of hook_entity_postsave, see EntityStorageBase::doPostSave()
2) Aha yes, I think we discussed moving it after the hooks.

Moved the hacky $hook_to_event_map to be a constant on the EntityEvents class. Also documented all of them, and reordered to be in the order they are usually fired.

TIL that class constants can only contain arrays as of PHP 5.6.0, and since our minimum requirements are 5.5.9, can't use that approach :(

Or, here's this. Which could be even better since another storage class could cheat and add their own event name...
Interdiff is against #28.

1) hook_entity_update and hook_entity_insert are the equivalent of hook_entity_postsave, see EntityStorageBase::doPostSave()

I like the idea of a postsave dispatcher. Too many times I faced the situation to implement both hook_entity_update() and hook_entity_insert() with the same code. In many cases you want the same reaction regardless if the entity is new or existing. IMO, the correct flow would be:

1. new entity: presave > save > postsave > insert
2. existing entity: presave > save > postsave

No update hook.

I opened #2798783: hook_entity_insert() and hook_entity_update() are confusing to discuss that, it's out of scope here IMO.

I think we should design the events solid from scratch, such that they are easy to grapsh. I don't think having a a 1:1 map to hooks is required here, instead we can take the opportunity and make sure the align well. That said, I'd just go for:
pre/postCreate
pre/postSave
pre/postDelete

Related: Once this is done, add onChange() for reactions. -> #1729812: Separate storage operations from reactions

FYI: This is what doctrine does:
http://docs.doctrine-project.org/projects/doctrine-orm/en/latest/referen...
They have persist for insert and update for update, but no common postSave.

1. +++ b/core/lib/Drupal/Core/Entity/EntityEvents.php
   @@ -0,0 +1,105 @@
   +   * This hook runs after a new entity object has just been instantiated.
   

   This is no hook any more that runs.

2. +++ b/core/lib/Drupal/Core/Entity/EntityEvents.php
   @@ -0,0 +1,105 @@
   +   * You can get the original entity object from $entity->original when it is an
   +   * update of the entity.
   

   Let's make sure the new API is solid from the start:
   $entity->original is no property of the entity. $original_entity is some temporary variable that is available during the operation. Imo, the event should provide that as second variable.

   ad #6:

   Refactoring $entity->original is a big task and definitely out of scope for this issue.
   There's an issue for it at #1480696: Move $entity->original to a separate hook argument.

   I'd agree that we should not re-factor it here, but we should define the API already such that we avoid unnecessary future changes. It's easy to add $entity->original to the event as another variable or as getter which just passes of to $entity->original for now. Then #1480696 can still do the re-factoring.

I agree with fago. If we want to provide good quality in the future we should think about the right names. We add a new API so we should not start with something we want to deprecate in the future anyway.

@tim.plunkett
What are your thoughts about that?

Seems like #2221347 (via #2798783) is in consensus that post-save is the right approach to replace hook_update and hook_insert. We just need to agree that the new Entity Event API is allowed to have non-parity with the old Entity Hook API.

But #1480696 is conflicted (and stalled 2+ years) about the right approach. If we want to keep this issue moving forward without postponing on 1480696, seems like Entity Event API needs to support ->original property.

also: bump

+++ b/core/lib/Drupal/Core/Entity/EntityEvents.php
@@ -0,0 +1,105 @@
+final class EntityEvents {
+
+  /**
+   * Maps entity hook names to event names.
+   *
+   * @var string[]
+   */
+  public static $hookToEventMap = [
+    'create' => self::CREATE,
+    'presave' => self::PRESAVE,
+    'insert' => self::INSERT,
+    'update' => self::UPDATE,
+    'predelete' => self::PREDELETE,
+    'delete' => self::DELETE,
+  ];

One thing I'd like to see is the ability to subscribe specifically to entity type-specific events. So there should be the ability to react to events like entity.create.node or entity.delete.user.

One possible approach is to add some static methods to this class which could generate the appropriate event names, so you could get the event name for deleting a user like this:

EntityEvents::delete('user')

The implementation could be as simple as:

return static::CREATE . '.' . $entity_type_id

@phenaproxima
Great idea!

Okay, here we go. This implements my idea from #38. No tests for that yet, though. And no interdiff, because #31 no longer applies to 8.4.x. Sorry!

+++ b/core/lib/Drupal/Core/Entity/EntityStorageBase.php
@@ -167,6 +167,23 @@ protected function invokeHook($hook, EntityInterface $entity) {
+      $event_name = call_user_func([EntityEvents::class, $hook], $this->entityTypeId);

The EntityEvents class seems a bit over the top. Can we make this a bit simpler?

Maybe just

// e.g. EntityEventNameBuilder::buildEventName($operation, $entityType) ==> 'entity.create.node'

public static function buildEventName($operation, $entityType) {
  return 'event.' . $operation . '.' . $entityType;
}

?

It is verbose, but consider how it will look when used:

public static function getSubscribedEvents() {
  return [
    EntityEvents::buildEventName('presave', 'view') => 'onViewPresave',
    EntityEvents::buildEventName('update', 'node') => 'onNodeUpdate',
  ];
}

vs.

public static function getSubscribedEvents() {
  return [
    EntityEvents::presave('view') => 'onViewPresave',
    EntityEvents::update('node') => 'onNodeUpdate',
  ];
}

IMHO the second way is a lot easier to read at a glance.

One possibility is to implement a magic __callStatic() method so that we can kind of have our cake and eat it too, but I don't know if that would be considered kosher. What do you think?

One possibility is to implement a magic __callStatic() method so that we can kind of have our cake and eat it too, but I don't know if that would be considered kosher. What do you think?

Once you have __call, you end up having no autocomplete support and what not. Are we scared of the code "duplication"?

Are we scared of the code "duplication"?

I am not, but I was trying to see if I could find an alternative that @kim.pepper might find more palatable.

Ah, it's a minor nit pick. I thought having an event name builder at all was over the top.

That's actually a good point. You have a string which will never change, why would need to have a method for that. There is not really much of an abstraction we have as a method.

The string does change, because the event name for inserting a node is not the same as inserting a user. It's a thin layer of abstraction, for sure, but it's worth having in case we need to implement more complex event name-building logic at some point in the future.

Also, we should not expect developers to remember the format of an event name for a specific entity type. I can easily see people getting confused about whether it's supposed to be EntityEvents::INSERT . ':node' or EntityEvents::INSERT . '.node'. Wrapping these event names in simple methods removes the ambiguity.

Not to mention that, should we need to change the event name at any point in the future, having a method gives us the latitude to do that easily.

Btw Here's related issue about event classes #2825358: Event class constants for event names can cause fatal errors when a module is installed

Re-roll.

Please look at:
https://www.drupal.org/sandbox/olexyymailsgmailcom/hook_manager

- it should be rather fast;
- it uses caching and static;
- priority is supported;
- we have progress in naming convention problems;
- we can inject services to classes that implement hooks;

+1 for #52

please please please can we get rid of enity hooks

reroll of #52

1. +++ b/core/modules/system/tests/modules/entity_test_event/src/EventSubscriber/TestEventSubscriber.php
   @@ -0,0 +1,44 @@
   +class TestEventSubscriber implements EventSubscriberInterface {
   ...
   +  public function onInsert(EntityEvent $event) {
   ...
   +  public function onUpdate(EntityEvent $event) {
   ...
   +  public function onDelete(EntityEvent $event) {
   

   Doc comments missing

2. +++ b/core/modules/system/tests/modules/entity_test_event/src/EventSubscriber/TestEventSubscriber.php
   @@ -0,0 +1,44 @@
   +  public static function getSubscribedEvents() {
   ...
   +  }
   +
   +
   +}
   

   2 lines after the method. should be 1

3. +++ b/core/tests/Drupal/KernelTests/Core/Entity/EntityEventsTest.php
   @@ -0,0 +1,64 @@
   +class EntityEventsTest extends KernelTestBase {
   ...
   +  public function testEventsInsert() {
   ...
   +  public function testEventsUpdate() {
   ...
   +  public function testEventsDelete() {
   

   Doc comments missing

Added comments/docblocks and addressed linter issues.

Looks good. +1 for RTBC

It still needs profiling & consensus on event names

An attempt to really use new API, still not clear how it should subscribe to entity-type-specific events
Probably event should have helper to extract hook name for BC

#64 for 8.6.x

Testbot

Just fixed coding standards for #64.

Just fixed coding standards for #66.

#70 for 8.7.x without changes in SqlContentEntityStorageTest.php

Interesting approach from sf-land https://romaricdrigon.github.io/2019/08/09/domain-events

Just to be referenced here, "there is a contrib for that": https://www.drupal.org/project/entity_events

Ofc, it would be better if this would be implemented/supported in core soon.

I wasn't aware of that project. What about a merge between the two projects?

Rerolled from #71

Trying to re-roll up to 9.1

Rerolled

Signature of EventDispatcherinterface::dispatch() has changed

Ignore

The 'core_version_requirement' constraint (9.1.0-dev) requires the 'core' key not be set in core/modules/system/tests/modules/entity_test_event/entity_test_event.info.yml 

The naming conventions adopted here probably need to allow for the fact that there is serious discussion about adding additional pre/post hooks that are OUTSIDE of the database transaction: #2992426: Add entity methods and hooks for executing pre/post code outside of the save transaction

We have been usting the patch in #85 for quite some time now with 0 problems.
Entities we have tested the patch on are core entities: Users, Comments, Taxonomy Terms | and commerce entities: Product, Variation, Attributes.

I will be back on this when/if we use it with more entities.

For now +1 for rtbc on this :P.

Not really sure what is happening with the PR and stuff, I have been using #85 for a long time and had no problems.

So for people like me, here is a reroll of #85 for 9.5.x.

#85 does not work for me when applied for 9.5.x. Does anyone has a patch that does? Thank you

#97 patch that was created for 10.x, worked for me for 9.5.x

Maybe helpful for someone to know

Fixed CCF for #97.
Please review.

Try To Fix The Failed CMF #97 Patch . And Add Interdiff between the #97-101.

Try To Fix The Failed CMF #101 Patch . And Add Interdiff between the #101-102

+++ b/core/tests/Drupal/Tests/Core/Entity/KeyValueStore/KeyValueEntityStorageTest.php
@@ -632,7 +632,9 @@
+/**
+ *
+ */

This could actually use some content. An empty PHPDoc doesn't add much value.

Legitimately curious, was #102 just an automated update without reviewing it? The empty comment is a code smell for a rote contribution.

That is quite possible indeed. I've seen a lot of re-rolls recently which were not good, omitting new files and things like adding comments that don't add any value.

Tried to fix CCF for #102.

Instead of removing the PHPDoc you should have added some content/text in it.

addressed the #107 Comment. upload The patch And Interdiff between.

Fix the Failed CMD Patch of Mine.Remove Failed Patch.

I have a project that uses a significant number of these hooks and it would be much nicer if they could be events instead.

What would also be good is if we could have a base EntityUpdateEvent (for all entity types), and extend that to NodeUpdateEvent (for nodes only), and subscribers on EntityUpdateEvent would also receive NodeUpdateEvent. Unfortunately, Symfony does not support this pattern as far as I can tell: https://github.com/symfony/symfony/pull/32079

I find it more challenging https://git.drupalcode.org/project/hux/-/blob/1.x/src/HuxReplacementHook...

@longwave please elaborate "these hooks" dowsides

@andypost all my code is written as services and so the hooks end up being

/**
 * Implements hook_node_update().
 */
function MODULE_node_update(NodeInterface $node) {
  if (in_array($node->bundle(), ['x', 'y'])) {
    \Drupal::service('MODULE.SERVICE')->method($node);
  }
}

It would be cleaner if I could just inject this service into an event listener.

There is a lot of activity around hooks via PHP attributes issue (#3366083: [META] Hooks via attributes on service methods (hux style)). I think it's time to make a final decision about which way we are going to replace the current hooks system.
So far we've got the following options.

1. Events via Symfony Event Dispatcher
2. Hooks via PHP attributes
3. Nothing (keep the current hook implementation)

This is required to allow starting work on concrete implementation of the new event/hook system. I am not sure on what is the best way to get the resolution. Guess it needs to be done by framework managers.

Forget !4340...

I moved the changes of !210 to 11.x in !4423
I will start working on the review of @claudiucristea, unless someone beats me to it, or the issue ends up getting dropped.
Having said that.. has there been any discussion on the matter?

Attaching a patch for 10.2.2 (Based on !4423) (Probably applies to older versions as well, that #97 does not cover)

This needs to be an MR against 11.x at this point.

This is fixed in #3442009: OOP hooks using attributes and event dispatcher :D

Moving to duplicate just so people don't think there was a commit here.

Is this really a duplicate? I don't think it is.

This issue is NOT about replacing hooks with events, it is about having events dispatched in addition to hooks so that an event-based programming model can be supported. Is that really being done in that other issue? There's no mention of that in the change record.

@tr the other issue is 266 comments long, it also has multiple related issues, including this one at 126 comments, did you read it/them before re-opening this one?

If you didn't read it, then why contradict the people who have read it and worked on this issue for the past decade without doing any research?

Yes I read it, I have been following it, and I don't see how it's a duplicate. Have *you* read this issue?

Oh, and I'm not allowed to ask a question when this issue I've been following for 9 years gets suddenly closed?

After reading through this issue and #3442009: OOP hooks using attributes and event dispatcher, as suggested in a comment above, it seems that there are two different approaches:

• Using attributes for hooks as is done in the hux module
• Using events alongside hooks as is done in the Hook Event Dispatcher module

These approaches do not seem to be mutually exclusive.

In #3442009: OOP hooks using attributes and event dispatcher, core has chosen to adopt the hux module approach.

This issue is about event dispatching as is done in the Hook Event Dispatcher module, whereas #3442009: OOP hooks using attributes and event dispatcher is about using PHP attributes to replace hooks (the change record states In a later Drupal version (Drupal 12 at earliest) we expect procedural hooks no longer to be supported and then these services and these LegacyHook marked shims will need to be removed.)

So if this issue is closed, my question is: has a decision been made to NOT support emitting events in addition to hooks? I originally started following this issue because I use Hook Event Dispatcher, which currently has over 16,000 other users. It would be nice to have a little more clarity regarding the "duplicate" status here.

So if this issue is closed, my question is: has a decision been made to NOT support emitting events in addition to hooks? I originally started following this issue because I use Hook Event Dispatcher, which currently has over 16,000 other users. It would be nice to have a little more clarity regarding the "duplicate" status here.

That's my question exactly, speaking from my perspective of the person who maintains Rules, which is 100% based on using Events.

With Rules, in D7, D8, D9, D10, D11.0, what we have to do is IMPLEMENT ENTITY CRUD HOOKS, then dispatch Events from within those hook implementations. Rules can of course integrate with ANY Event from ANY source, but Entity CRUD events are a main use-case.

Likewise, Hook Event Dispatcher and the Entity Events modules (and others) do something similar to Rules. With the difference that Rules doesn't just do Events, but also Conditions and Actions so you don't need to write your own Event Subscriber. But that's another topic ...

I was hoping this issue would result in a cleaner integration with core, where CORE would dispatch the Events. That would fill a very common use-case for workflows, and in my mind that is why this issue was opened. I can't speak for the original posters, but @bojanz has always been involved with Commerce and @fago of course it the author of Rules, so I'm guessing that their motivations in pursing this issue are similar to mine and @ptmkenny's. From the OP, "Now that we have our own reasonably performant EventDispatcher, there is no reason not to fire an event every time the matching hook is fired."

BEFORE I re-opened this issue in #126 I read the ENTIRE body of #3442009: OOP hooks using attributes and event dispatcher. But more importantly, I read the CODE to see EXACTLY what that issue was doing. Because Change Records are notoriously sparse in details.

NOW, however, I have in addition actually implemented Hooks using the new system - I have updated about 10 modules (including Rules) with about 50 hooks in total, with full testing, so I now have practical knowledge and experience in how to properly implement Hooks with the new system. And I can now say 100% that this issue IS NOT A DUPLICATE OF #3442009: OOP hooks using attributes and event dispatcher. Specifically, I STILL have to implement the Entity CRUD hooks with the new system, and I STILL have to create and dispatch Events from these hooks. Basically nothing has changed from the perspective of a contributed module developer who wants to program with events.

And yes, I've tried to subscribe to the new, internal non-events named "drupal_hook.$hook", but that cannot be done unless you're implementing a Hook using the Hook attribute. Any other subscriber throws PHP errors.

So again, why the pushback and the hate from my observation that this issue should not be closed as a duplicate? It's NOT a duplicate. Perhaps there are other issues that duplicate this one, but #3442009: OOP hooks using attributes and event dispatcher is not one of them.

This is not a judgement of the work done in #3442009: OOP hooks using attributes and event dispatcher. If you like I will offer you proper obeisance for the hard work in that issue. But really, that's not the point here. Regardless of what was done there, that does not fix the issue HERE.

At the risk of being shunned, I'm going to re-open this issue. I would like to at least have the OPTION for core to dispatch Events - doing it in conjunction with Hooks is an obvious thing to do, especially now that Hooks are piggybacking off of the Symfony Event dispatcher. Dispatching events at the same time as invoking hooks is almost no overhead - Drupal is already determining whether there are listeners before calling the hooks, so all that needs to be done is to create Event objects (a few hundred bytes each) and dispatch these events to the listeners. Almost no overhead, yet it enables many use cases that have never been well-served by the core Drupal event model.

@catch, @nicxvan, Please explain why you think this IS a duplicate issue. It seems to me you made a mistake in closing it.

And, @catch, if you've even bothered to read this far:

If you didn't read it, then why contradict the people who have read it and worked on this issue for the past decade without doing any research?

You seem to think I'm disrespecting the people who worked on that issue. That's not the case, because I never said anything bad (or good) about that issue. I simply said it is NOT A DUPLICATE of what is proposed here. And frankly, YOU are disrespecting ME by totally ignoring my 17 year long contribution to Drupal and by making a snap judgement and accusation without reasonable consideration of and refutation of my argument that this is not a duplicate. At least do me the favor of addressing my concern directly rather than dismissing it outright with no explanation.

And BTW, it's "TR" not "tr". Don't reduce me to a machine name.

I think NW is more appropriate as we have a pull request (which needs a rebase) and some other tags which justify NW.

Also let's not get too emotional here. It's sometimes difficult to express sentiment in text. I think the real issue is that #124 was taken for granted without reading in detail (lot's of reading) both issues.

Let's continue in a positive way, as I also don't believe this is a duplicate, and I'm also using this approach on a few projects.

A decision was made to update the hook system to support methods on services, so that we don't have to convert everything to events. Drupal core will IMHO not add support for events and hooks for the same thing. Performance is one thing, but I think the bigger reason is cognitive load for developers, having to learn two different ways to do the same thing and two different ways to find and search hook implementations adds considerable complexity. And, there are fairly frequent use cases where some implementations need to be called in a certain order, supporting hooks and events would likely make that even harder if not impossible to mainain.

I understand that there are use cases that might be easier with an event subscriber, but I don't think it's the end of the world to add a few lines of glue code that passes the handful of fixed entity hooks through to Rules, a generic implementation might be possible with a decorated module handler service or so, but I didn't verify or think about whether or not that is useful.

Maybe won't fix is a better status, so lets go with that. Either way, the issue has now been closed by both a core maintainer and an entity system maintainer, so lets leave it at that.

PS: The drupal.org login process changes have lowercased everyones username, mine was also changed from Berdir to berdir. I am certain that using @tr wasn't meant to be disrespectful in any way, it's just your username now.

Re the username. There is now an KeyCloak feature request open for this: https://github.com/keycloak/keycloak/issues/32869

Re #134: Good! I usually use class resolver in hooks and that adds needless boilerplate. I think using services for hooks may also be annoying as some modules implement a lot of hooks so maybe we could just check if class exists in certain namespace, register and cache if it does and use the class resolver to instantiate? Do we already have an issue for that?

Just commenting here to maybe clear something up as well.

The OOP change makes all hooks event listeners, so if you need to you can use a service provider to tag services with the event listener tag for the drupal_hook.$event tag yourself, dynamically.

Event listeners are a little different than event subscribers, but you should be able to interact with specific hooks as needed.
https://symfony.com/doc/current/event_dispatcher.html

Also @graber, can you share an example?

hey, how about hooks like field_values_init or translation_create. The second one is used by the path module here... and since this hook is not in the EntityEvents::$hookToEventMap array it's never being executed.

Because of that, I have a problem with saving URL aliases for translatable content (when I save a translation, its URL alias is not saved, it's empty after saving the translation, although it shows in the form).

I made a quick fix for that:

// Dispatch an event for the entity-level hook (i.e., hook_entity_$hook).
if (isset(EntityEvents::$hookToEventMap[$hook])) {
  $event_dispatcher->dispatch($event, EntityEvents::$hookToEventMap[$hook]);
} else {
  $this->moduleHandler()->invokeAll('entity_' . $hook, [$entity]);
}

... and it helped in my case, but I'm not sure if it's a correct way to go.

I'm using Drupal 10.3.9.

We are re-attaching our patch here for two reasons:

1. Maintenance on Drupal 10 and composer-patches ^2.0
   
   We are moving to cweagans/composer-patches 2.x.
   We need a single, well-defined patch that applies cleanly to the core version we use.
   Having it referenced in this issue helps us keep the patch in sync and document why we use it.
2. Hope for future core support
   
   This approach was not accepted for core in this issue, but we still rely on it.
   We hope that a similar solution (events for entity create/presave/insert/update/predelete/delete)
   will be considered for a future Drupal version, so that projects like ours can avoid maintaining a large patch.

What this patch provides

It adds Symfony-style events for the same lifecycle points that already have hooks
(hook_entity_insert, hook_ENTITY_TYPE_presave, etc.).
That allows using EventSubscriber with priorities, keeps logic out of hooks,
and makes it easier to test and extend.

Our project uses these events for many entity types
(e.g. store orders, draft orders, services, routes, passengers);
without this patch we would have to rely only on hook implementations
and lose the event-based design.

RE #139
I would recommend to not maintain this patch and instead change your project to use #[Hook('entity_')] attributes.

For example compare the test implementation from the MR to the test implementation of the attribute.

All you have to do is:

1. Move your class to the Hook folder
2. Add the attribute to the method
3. remove the event subscriber boiler plate

Reverting tags

-Patch for 10.3.x / Drupal 10: entity events (re-upload for composer-patches ^2.0 and future reference)