8000 merged branch jalliot/event-subscriber (PR #2021) · Faianca/symfony@b3ea939 · GitHub
[go: up one dir, main page]
Skip to content

Commit b3ea939

Browse files
fabpotfabpot
committed
merged branch jalliot/event-subscriber (PR symfony#2021)
Commits ------- 3223c5a Removed now useless test 21cf0ac Backported new behaviour from PR symfony#2148 and removed check for interface at run-time 8b240d4 Implementation of kernel.event_subscriber tag for services. Discussion ---------- Added missing kernel.event_subscriber tag (closes symfony#2012) This PR adds a ``kernel.event_subscriber`` tag which allows to register services as event subscribers in the same way ``kernel.event_listener`` allows to register them as event listeners. The service is still lazy loaded and the DIC does not need to be recompiled for every modification in the service's code. There is one important thing to remember: If the service is created by a factory, the class parameter **MUST** reflect the real class of the service, although it is not needed at the moment for the DIC. For that issue, we could either forbid services created by factories or add a note to the documentation. This PR closes symfony#2012. --------------------------------------------------------------------------- by jalliot at 2011/08/24 06:42:18 -0700 I'm not sure the test is good enough so feel free to add some more. --------------------------------------------------------------------------- by jalliot at 2011/08/25 03:46:20 -0700 I re-implemented the check for EventSubscriberInterface in ContainerAwareEventDispatcher because I think the overhead is minimum and it allows to use this method even without the tag (at run-time). I also added some tests for RegisterKernelListenersPass. --------------------------------------------------------------------------- by stof at 2011/09/04 02:42:00 -0700 @jalliot Your branch conflicts with the current master. could you rebase it ? --------------------------------------------------------------------------- by jalliot at 2011/09/04 02:57:03 -0700 Rebased --------------------------------------------------------------------------- by jalliot at 2011/09/13 02:19:46 -0700 @fabpot What do you think about this PR? At the moment, the subscribers are not really usable in Symfony2 because of the lack of this tag. --------------------------------------------------------------------------- by fabpot at 2011/09/13 04:17:46 -0700 I don't like subscribers. There are other PRs on adding more support for them, but the reality is that they are complex for no added benefit. I'm wondering if it wouldn't be better to just remove them altogether. --------------------------------------------------------------------------- by jalliot at 2011/09/13 04:38:20 -0700 @fabpot Well I prefer listeners too but I think that if Symfony2 does support subscribers (which it does at the moment), it should do it properly and completely, thus allowing to register subscriber services like here or to register several methods for one same event like in symfony#2148. But I guess that if you merged those 2 PRs (well actually this one would have to be modified first if symfony#2148 is merged but I'll do it then), many use cases would be covered and people should stop asking for more support :) (except maybe for removing the static modifier but this would be wrong IMO and prevent entirely this PR). --------------------------------------------------------------------------- by fabpot at 2011/09/28 11:47:10 -0700 @jalliot: symfony#2148 has been merged. Can you update this PR accordingly? thanks. --------------------------------------------------------------------------- by jalliot at 2011/09/28 12:00:44 -0700 Sure thing. Will do it as well as removing the check for the interface tonight or tomorrow :) --------------------------------------------------------------------------- by jalliot at 2011/09/29 08:53:17 -0700 @fabpot Check for interface removed and symfony#2148 merged. Also rebased on latest master. --------------------------------------------------------------------------- by fabpot at 2011/09/29 09:09:11 -0700 Tests do not pass. --------------------------------------------------------------------------- by jalliot at 2011/09/29 09:18:48 -0700 @fabpot Fixed
2 parents 245ff6d + 3223c5a commit b3ea939

File tree

4 files changed

+159
-0
lines changed

4 files changed

+159
-0
lines changed

‎src/Symfony/Bundle/FrameworkBundle/ContainerAwareEventDispatcher.php

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
*
2222
* @author Fabien Potencier <fabien@symfony.com>
2323
* @author Bernhard Schussek <bernhard.schussek@symfony.com>
24+
* @author Jordan Alliot <jordan.alliot@gmail.com>
2425
*/
2526
class ContainerAwareEventDispatcher extends EventDispatcher
2627
{
@@ -103,6 +104,27 @@ public function getListeners($eventName = null)
103104
return parent::getListeners($eventName);
104105
}
105106

107+
/**
108+
* Adds a service as event subscriber
109+
*
110+
* @param string $serviceId The service ID of the subscriber service
111+
* @param string $class The service's class name (which must implement EventSubscriberInterface)
112+
*/
113+
public function addSubscriberService($serviceId, $class)
114+
{
115+
foreach ($class::getSubscribedEvents() as $eventName => $params) {
116+
if (is_string($params)) {
117+
$this->listenerIds[$eventName][] = array($serviceId, $params, 0);
118+
} elseif (is_string($params[0])) {
119+
$this->listenerIds[$eventName][] = array($serviceId, $params[0], $params[1]);
120+
} else {
121+
foreach ($params as $listener) {
122+
$this->listenerIds[$eventName][] = array($serviceId, $listener[0], isset($listener[1]) ? $listener[1] : 0);
123+
}
124+
}
125+
}
126+
}
127+
106128
/**
107129
* {@inheritDoc}
108130
*

‎src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/RegisterKernelListenersPass.php

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,5 +43,18 @@ public function process(ContainerBuilder $container)
4343
$definition->addMethodCall('addListenerService', array($event['event'], array($id, $event['method']), $priority));
4444
}
4545
}
46+
47+
foreach ($container->findTaggedServiceIds('kernel.event_subscriber') as $id => $attributes) {
48+
// We must assume that the class value has been correcly filled, even if the service is created by a factory
49+
$class = $container->getDefinition($id)->getClass();
50+
51+
$refClass = new \ReflectionClass($class);
52+
$interface = 'Symfony\Component\EventDispatcher\EventSubscriberInterface';
53+
if (!$refClass->implementsInterface($interface)) {
54+
throw new \InvalidArgumentException(sprintf('Service "%s" must implement interface "%s".', $id, $interface));
55+
}
56+
57+
$definition->addMethodCall('addSubscriberService', array($id, $class));
58+
}
4659
}
4760
}

‎src/Symfony/Bundle/FrameworkBundle/Tests/ContainerAwareEventDispatcherTest.php

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use Symfony\Component\DependencyInjection\Container;
1515
use Symfony\Bundle\FrameworkBundle\ContainerAwareEventDispatcher;
1616
use Symfony\Component\EventDispatcher\Event;
17+
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
1718
use Symfony\Component\DependencyInjection\Scope;
1819

1920
class ContainerAwareEventDispatcherTest extends \PHPUnit_Framework_TestCase
@@ -39,6 +40,27 @@ public function testAddAListenerService()
3940
$dispatcher->dispatch('onEvent', $event);
4041
}
4142

43+
public function testAddASubscriberService()
44+
{
45+
$event = new Event();
46+
47+
$service = $this->getMock('Symfony\Bundle\FrameworkBundle\Tests\SubscriberService');
48+
49+
$service
50+
->expects($this->once())
51+
->method('onEvent')
52+
->with($event)
53+
;
54+
55+
$container = new Container();
56+
$container->set('service.subscriber', $service);
57+
58+
$dispatcher = new ContainerAwareEventDispatcher($container);
59+
$dispatcher->addSubscriberService('service.subscriber', 'Symfony\Bundle\FrameworkBundle\Tests\SubscriberService');
60+
61+
$dispatcher->dispatch('onEvent', $event);
62+
}
63+
4264
public function testPreventDuplicateListenerService()
4365
{
4466
$event = new Event();
@@ -174,3 +196,16 @@ function onEvent(Event $e)
174196
{
175197
}
176198
}
199+
200+
class SubscriberService implements EventSubscriberInterface
201+
{
202+
static function getSubscribedEvents() {
203+
return array(
204+
'onEvent' => 'onEvent',
205+
);
206+
}
207+
208+
function onEvent(Event $e)
209+
{
210+
}
211+
}

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/RegisterKernelListenersPassTest.php

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\Compiler;
13+
14+
use Symfony\Component\DependencyInjection\ContainerBuilder;
15+
use Symfony\Component\DependencyInjection\Definition;
16+
use Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\RegisterKernelListenersPass;
17+
18+
class RegisterKernelListenersPassTest extends \PHPUnit_Framework_TestCase
19+
{
20+
/**
21+
* Tests that event subscribers not implementing EventSubscriberInterface
22+
* trigger an exception.
23+
*
24+
* @expectedException \InvalidArgumentException
25+
*/
26+
public function testEventSubscriberWithoutInterface()
27+
{
28+
// one service, not implementing any interface
29+
$services = array(
30+
'my_event_subscriber' => array(0 => array()),
31+
);
32+
33+
$definition = $this->getMock('Symfony\Component\DependencyInjection\Definition');
34+
$definition->expects($this->atLeastOnce())
35+
->method('getClass')
36+
->will($this->returnValue('stdClass'));
37+
38+
$builder = $this->getMock('Symfony\Component\DependencyInjection\ContainerBuilder');
39+
$builder->expects($this->any())
40+
->method('hasDefinition')
41+
->will($this->returnValue(true));
42+
43+
// We don't test kernel.event_listener here
44+
$builder->expects($this->atLeastOnce())
45+
->method('findTaggedServiceIds')
46+
->will($this->onConsecutiveCalls(array(), $services));
47+
48+
$builder->expects($this->atLeastOnce())
49+
->method('getDefinition')
50+
->will($this->returnValue($definition));
51+
52+
$registerListenersPass = new RegisterKernelListenersPass();
53+
$registerListenersPass->process($builder);
54+
}
55+
56+
public function testValidEventSubscriber()
57+
{
58+
$services = array(
59+
'my_event_subscriber' => array(0 => array()),
60+
);
61+
62+
$definition = $this->getMock('Symfony\Component\DependencyInjection\Definition');
63+
$definition->expects($this->atLeastOnce())
64+
->method('getClass')
65+
->will($this->returnValue('Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\Compiler\SubscriberService'));
66+
67+
$builder = $this->getMock('Symfony\Component\DependencyInjection\ContainerBuilder');
68+
$builder->expects($this->any())
69+
->method('hasDefinition')
70+
->will($this->returnValue(true));
71+
72+
// We don't test kernel.event_listener here
73+
$builder->expects($this->atLeastOnce())
74+
->method('findTaggedServiceIds')
75+
->will($this->onConsecutiveCalls(array(), $services));
76+
77+
$builder->expects($this->atLeastOnce())
78+
->method('getDefinition')
79+
->will($this->returnValue($definition));
80+
81+
$registerListenersPass = new RegisterKernelListenersPass();
82+
59C1 $registerListenersPass->process($builder);
83+
}
84+
}
85+
86+
class SubscriberService implements \Symfony\Component\EventDispatcher\EventSubscriberInterface
87+
{
88+
static function getSubscribedEvents() {}
89+
}

0 commit comments

Comments
 (0)
0