Events namespaces should be aware of class inheritance

[JanTvrdik]

Example: Nextras\Orm\Repository\Repository defines many events, such as onAfterInsert.

In DI container I have multiple instances of Nextras\Orm\Repository\Repository, e.g. AuthorsRepository and BooksRepository. I want to be able to listen to onAfterInsert event on a specific repository, however listening to AuthorsRepository::onAfterInsert does not work. This can be easily fixed by changing $property->getDeclaringClass() to $class. It would however also be nice to be able to listen to all repositories with event Nextras\Orm\Repository\Repository::onAfterInsert. This can be done (probably) by rewriting optimizeListeners method.

What do you think about it?

[fprochazka]

#25, #44 :)

[JanTvrdik]

I still don't understand why do you think that it is a bad design Could you elaborate?. cc @hrach

[fprochazka]

If you extend a class to switch the implementation, you break the application, because all the events on that class would change. I think it's better, if all the autowired events are called on the class that they are defined on.

If you need something dynamic, passing an event manager and calling

$this->evm->dispatchEvent(
    get_class($this) . '::onLoad', 
    new Kdyby\Events\EventArgsList(array($this, $entita))
);

is way better than adding new syntaxes or another magic.

---

On the other hand, I may be willing to figure out an annotation for example, that would change the behaviour of some specific event. Bot I won't change the default behaviour, because that would be a massive BC Break.

[JanTvrdik]

I don't understand how that would break anything. All previously subscribed events would work.

[JanTvrdik]

Prototype patch backup

From 761a04607c090deb77fbebf268cc70fa543b64fa Mon Sep 17 00:00:00 2001
From: Jan Tvrdik
Date: Fri, 3 Oct 2014 16:44:53 +0200
Subject: [PATCH] kdyby/events hack

---
 vendor/kdyby/events/src/Kdyby/Events/DI/EventsExtension.php | 12 +++++++++++-
 vendor/kdyby/events/src/Kdyby/Events/EventManager.php       |  9 ++++++++-
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/vendor/kdyby/events/src/Kdyby/Events/DI/EventsExtension.php b/vendor/kdyby/events/src/Kdyby/Events/DI/EventsExtension.php
index ad3a13b..b43a3c7 100644
--- a/vendor/kdyby/events/src/Kdyby/Events/DI/EventsExtension.php
+++ b/vendor/kdyby/events/src/Kdyby/Events/DI/EventsExtension.php
@@ -305,7 +305,7 @@ class EventsExtension extends Nette\DI\CompilerExtension

            $def->addSetup('$' . $name, array(
                new Nette\DI\Statement($this->prefix('@manager') . '::createEvent', array(
-                   array($property->getDeclaringClass()->getName(), $name),
+                   array($class->getName(), $name),
                    new Code\PhpLiteral('$service->' . $name)
                ))
            ));
@@ -326,10 +326,20 @@ class EventsExtension extends Nette\DI\CompilerExtension
                $listeners[$eventName][] = $serviceName;
                if ($namespace !== NULL) {
                    $listeners[$event][] = $serviceName;
+                   foreach ($builder->getDefinitions() as $def) {
+                       $class = $def->getClass();
+                       if (is_subclass_of($class, $namespace)) {
+                           $listeners["$class::$event"][] = $serviceName;
+                       }
+                   }
                }
            }
        }

+       foreach ($listeners as $id => $subscribers) {
+           $listeners[$id] = array_unique($subscribers);
+       }
+
        $builder->getDefinition($this->prefix('manager'))
            ->setClass('Kdyby\Events\LazyEventManager', array($listeners))
            ->setup = $this->allowedManagerSetup;
diff --git a/vendor/kdyby/events/src/Kdyby/Events/EventManager.php b/vendor/kdyby/events/src/Kdyby/Events/EventManager.php
index 3ac5e80..b1cacad 100644
--- a/vendor/kdyby/events/src/Kdyby/Events/EventManager.php
+++ b/vendor/kdyby/events/src/Kdyby/Events/EventManager.php
@@ -337,7 +337,14 @@ class EventManager extends Doctrine\Common\EventManager
            }

        } else {
-           $available = !empty($this->listeners[$event][$namespace]) ? $this->listeners[$event][$namespace] : array();
+           $available = array();
+           do {
+               if (!empty($this->listeners[$event][$namespace])) {
+                   foreach ($this->listeners[$event][$namespace] as $callback) {
+                       $available[] = $callback;
+                   }
+               }
+           } while ($namespace = get_parent_class($namespace));
        }

        if (empty($available)) {
-- 
1.9.4.msysgit.1

[fprochazka]

Oh, I see now. You would emit the event from the concrete class and also from all the parent classes? That's actually not a bad idea.

Would you please open a pullrequest (ideally with a test case), so we can see if that would work?