Move entity_invoke_bundle_hook() to EntityManager

While renaming I found that BlockContentType executed this hook twice ;)

What if we don't use the word hook in the method, so we can decide to extend this with Symfony event support later?

@Xano makes sense, so any suggestions?
notifyBundleEvent invokeBundle

I think event is good. It's a generic term that applies to both event hooks (as opposed to info hooks) and Symfony events.

It is not misleading. Hooks are our technical solution for interacting with modules, whether for events or for gathering metadata (info hooks). We're still talking about CRUD events here, and the way the interaction takes place is secondary to that.

On the other hand, using an on prefix would work too.

@yched do you mean to split this into 3 methods?
This could be onBundleOperation() single method

#1498720: [meta] Make the entity storage system handle changes in the entity and field schema definitions goes with methods named like onFieldStorageDefinitionCreate(). We should make sure our naming pattern is consistent here. But, should the naming reflect that this is not just a regular event listeners but actually omitts the event as well? Having onFieldStorageDefinitionCreate() as method name for the method that calls other on* seems confusing, so a name-separation seems to make sense.

Then, maybe handleFoo() would be a good name for actually reacting + omitting the event/hook?

"Event" is misleading as long as this is still hook-based, IMO.

Agreed - it would be strange to have no event then. I'd be fine with adding an event in addition to the hook foo. If we can agree on that + open a follow-up to do it we could go with the new naming already.

-> invokeFooEvent() or dispatchFooEvent() ?

Here's a split

But, should the naming reflect that this is not just a regular event listeners but actually omitts the event as well? Having onFieldStorageDefinitionCreate() as method name for the method that calls other on* seems confusing, so a name-separation seems to make sense.

I don't understand the concern here. I don't see any problem in EM::onFoo() calling ES::onFoo(). In other words, I like what's in #10. In fact, I'm tempted to say that all code outside of the entity manager that calls $storage->on*() should call $manager->on*() instead, and let EM always delegate those kinds of reactions to ES, but that's a separate issue.

1. +++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityBundleBase.php
   @@ -38,7 +38,7 @@ public static function postDelete(EntityStorageInterface $storage, array $entiti
   +      \Drupal::entityManager()->onBundleDelete($entity->getEntityType()->getBundleOf(), $entity->id());
   

   Can we use $this->entityManager() here instead?

2. +++ b/core/lib/Drupal/Core/Entity/EntityManager.php
   @@ -973,4 +973,51 @@ public function getEntityTypeFromClass($class_name) {
   +    if (method_exists($storage, $method)) {
   

   How about changing this to if ($storage instanceof FieldableEntityStorageInterface)?

3. +++ b/core/lib/Drupal/Core/Entity/EntityManager.php
   @@ -973,4 +973,51 @@ public function getEntityTypeFromClass($class_name) {
   +    // Invoke hook_entity_bundle_rename() hook.
   +    $this->moduleHandler->invokeAll('entity_bundle_delete', array($entity_type_id, $bundle));
   

   Comment referring to wrong hook name.

+++ b/core/modules/entity/entity.module
@@ -103,12 +103,13 @@ function entity_entity_bundle_delete($entity_type_id, $bundle) {
-  foreach (\Drupal::entityManager()->getDefinitions() as $entity_type_id => $entity_type) {
+  foreach ($entity_manager->getDefinitions() as $entity_type_id => $entity_type) {
     if ($entity_type->getProvider() == $module) {
-      foreach (array_keys(entity_get_bundles($entity_type_id)) as $bundle) {
-        entity_invoke_bundle_hook('delete', $entity_type_id, $bundle);
+      foreach (array_keys($entity_manager->getBundleInfo($entity_type_id)) as $bundle) {
+        $entity_manager->onBundleDelete($entity_type_id, $bundle);
       }
     }
   }

I'm wondering if this is called twice now in many cases because config entities are explicitly deleted as well? Not sure if that happens before or after...

Chasing HEAD
Q: placed code for "base_field_override" into EM, probably better to move to hook entity_entity_bundle_rename() where entity displays are updated

#11
1) no, because the method is static
2) nice idea, fixed
3) fixed

#12 that was added in #1199946: Disabled modules are broken beyond repair so the "disable" functionality needs to be removed so patch to try to remove

once again - removal of entity_module_preuninstall()

@Berdir there could be a reason for the hook - entity test does not have config entity to define its bundles... but this code looks fragile

EDIT The only case I have in mind - bundles' entities are provided by other module then deinstalled enity type

should wait for #2031717: Make entity module not required and needs revert to #13

rerolled

This is consistent with onEntityTypeCreate() and so on that we have no, looks good to me.

The function is new in 8.x and already has a change record, we can just update https://www.drupal.org/node/1964766 when it is committed.

We should also add a note about ConfigEntityBundleBase while we update it, saying that when using that, this is handled automatically for this.

Committed 9617c20 and pushed to 8.0.x. Thanks!

Time to update https://www.drupal.org/node/1964766

Please review a changes https://www.drupal.org/node/1964766/revisions/view/2640104/7611839

looks good to me