Skip to content

Commit c1a2804

Browse files
njoubert-cleveragenjoubert-cleverage
authored
Merge pull request #196 from cleverage/189
#189 - Fix EventDispatcherTask ignoring the event_name option
2 parents e6a424a + 06dd728 commit c1a2804

5 files changed

Lines changed: 188 additions & 13 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ Latest
88
## Fixes
99
* [#192](https://github.com/cleverage/process-bundle/issues/192) Fix CommandRunnerTask: only pass the `options` option to `Process::setOptions()`, support string `commandline` through `Process::fromShellCommandline()`, validate option types. Update documentation, add tests.
1010
* [#194](https://github.com/cleverage/process-bundle/issues/194) Fix ProcessLauncherTask: the `process_options` normalizer returned an array despite its scalar return type, so the task always failed with a `TypeError`. Update documentation, add tests.
11+
* [#189](https://github.com/cleverage/process-bundle/issues/189) Fix EventDispatcherTask: dispatch the event under the `event_name` option (regression since v4.0). `event_name` is now optional: when `null` (default), the event is dispatched under its class name. Update documentation, add tests.
12+
13+
## Deprecated
14+
* [#189](https://github.com/cleverage/process-bundle/issues/189) EventDispatcherTask: when `event_name` is set, listening to `CleverAge\ProcessBundle\Event\EventDispatcherTaskEvent` is deprecated (the event is still dispatched under its class name, with an `E_USER_DEPRECATED` error, if it has listeners). Listen to the configured `event_name` instead: the BC layer will be removed in v6.0.
1115

1216
v5.0
1317
-----

‎docs/04-advanced_workflow.md‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -238,9 +238,8 @@ class ProcessFailureListener
238238

239239
You can also use the [EventDispatcherTask](reference/tasks/event_dispatcher_task.md) to trigger an event in the middle
240240
of a process: it dispatches a `CleverAge\ProcessBundle\Event\EventDispatcherTaskEvent`, giving access to the current
241-
`ProcessState`. Note that in the current implementation the event is dispatched under its class name
242-
(`CleverAge\ProcessBundle\Event\EventDispatcherTaskEvent`): the `event_name` option is required but not passed to the
243-
event dispatcher.
241+
`ProcessState`. The event is dispatched under the `event_name` option or, when it is not set, under its class name
242+
(`CleverAge\ProcessBundle\Event\EventDispatcherTaskEvent`).
244243

245244
## Parallelization
246245

‎docs/reference/tasks/event_dispatcher_task.md‎

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,10 @@ Possible outputs
2424
Options
2525
-------
2626

27-
| Code | Type | Required | Default | Description |
28-
|--------------|----------|:--------:|---------|---------------------------------------------------------------|
29-
| `event_name` | `string` | **X** | | Name of the event (see Notes: currently not used to dispatch) |
30-
| `passive` | `bool` | | `true` | If `true`, the input is passed to the output before dispatch |
27+
| Code | Type | Required | Default | Description |
28+
|--------------|----------------|:--------:|---------|------------------------------------------------------------------------------|
29+
| `event_name` | `string\|null` | | `null` | Name of the dispatched event, `null` to use the event class name (see Notes) |
30+
| `passive` | `bool` | | `true` | If `true`, the input is passed to the output before dispatch |
3131

3232
Examples
3333
--------
@@ -50,6 +50,24 @@ push_data_event:
5050
Notes
5151
-----
5252
53-
The event is dispatched without an explicit name (`$eventDispatcher->dispatch($event)`), so its name is the event
54-
class name. Listeners must therefore subscribe to `CleverAge\ProcessBundle\Event\EventDispatcherTaskEvent`; the
55-
`event_name` option is required and validated but not used for dispatching.
53+
The event is dispatched with `$eventDispatcher->dispatch($event, $eventName)`:
54+
55+
* when `event_name` is set, listeners must subscribe to that name:
56+
57+
```php
58+
#[AsEventListener(event: 'myapp.data_queue')]
59+
public function onDataQueue(EventDispatcherTaskEvent $event): void
60+
{
61+
$input = $event->getState()->getInput();
62+
}
63+
```
64+
65+
* when `event_name` is `null`, the event name is the event class name, so listeners must subscribe to
66+
`CleverAge\ProcessBundle\Event\EventDispatcherTaskEvent`. Every `EventDispatcherTask` without `event_name` then
67+
triggers the same listeners.
68+
69+
**Deprecated since v5, removed in v6.0**: from v4.0 to v5.0, the `event_name` option was ignored and the event was
70+
always dispatched under its class name. To preserve backward compatibility, when `event_name` is set (and differs from the
71+
event class name) and listeners subscribe to `CleverAge\ProcessBundle\Event\EventDispatcherTaskEvent`, the event is also
72+
dispatched under its class name and an `E_USER_DEPRECATED` error is triggered. In v6.0, listen to the configured
73+
`event_name` instead, or remove the `event_name` option.

‎src/Task/Event/EventDispatcherTask.php‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,14 +39,31 @@ public function execute(ProcessState $state): void
3939

4040
$event = new EventDispatcherTaskEvent($state);
4141

42-
$this->eventDispatcher->dispatch($event);
42+
$this->eventDispatcher->dispatch($event, $options['event_name']);
43+
44+
// @deprecated BC layer since v5, remove me in v6.0: from v4.0 to v5.0, the event was only dispatched under its
45+
// class name, even when event_name was set
46+
if (null !== $options['event_name']
47+
&& EventDispatcherTaskEvent::class !== $options['event_name']
48+
&& $this->eventDispatcher->hasListeners(EventDispatcherTaskEvent::class)
49+
) {
50+
@trigger_error(
51+
\sprintf(
52+
'Listening to "%s" for an EventDispatcherTask with the "event_name" option set is deprecated since v5 and will not work anymore in v6.0, listen to "%s" instead.',
53+
EventDispatcherTaskEvent::class,
54+
$options['event_name'],
55+
),
56+
\E_USER_DEPRECATED
57+
);
58+
$this->eventDispatcher->dispatch($event);
59+
}
4360
}
4461

4562
protected function configureOptions(OptionsResolver $resolver): void
4663
{
47-
$resolver->setRequired(['event_name']);
64+
$resolver->setDefault('event_name', null);
4865
$resolver->setDefault('passive', true);
49-
$resolver->setAllowedTypes('event_name', ['string']);
66+
$resolver->setAllowedTypes('event_name', ['null', 'string']);
5067
$resolver->setAllowedTypes('passive', ['boolean']);
5168
}
5269
}

‎tests/Task/Event/EventDispatcherTaskTest.php‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/*
6+
* This file is part of the CleverAge/ProcessBundle package.
7+
*
8+
* Copyright (c) Clever-Age
9+
*
10+
* For the full copyright and license information, please view the LICENSE
11+
* file that was distributed with this source code.
12+
*/
13+
14+
namespace CleverAge\ProcessBundle\Tests\Task\Event;
15+
16+
use CleverAge\ProcessBundle\Configuration\ProcessConfiguration;
17+
use CleverAge\ProcessBundle\Configuration\TaskConfiguration;
18+
use CleverAge\ProcessBundle\Context\ContextualOptionResolver;
19+
use CleverAge\ProcessBundle\Event\EventDispatcherTaskEvent;
20+
use CleverAge\ProcessBundle\Model\AbstractConfigurableTask;
21+
use CleverAge\ProcessBundle\Model\ProcessHistory;
22+
use CleverAge\ProcessBundle\Model\ProcessState;
23+
use CleverAge\ProcessBundle\Task\Event\EventDispatcherTask;
24+
use PHPUnit\Framework\TestCase;
25+
use Symfony\Component\EventDispatcher\EventDispatcher;
26+
use Symfony\Component\OptionsResolver\Exception\InvalidOptionsException;
27+
28+
#[\PHPUnit\Framework\Attributes\CoversClass(EventDispatcherTask::class)]
29+
#[\PHPUnit\Framework\Attributes\UsesClass(EventDispatcherTaskEvent::class)]
30+
#[\PHPUnit\Framework\Attributes\UsesClass(AbstractConfigurableTask::class)]
31+
#[\PHPUnit\Framework\Attributes\UsesClass(ProcessConfiguration::class)]
32+
#[\PHPUnit\Framework\Attributes\UsesClass(TaskConfiguration::class)]
33+
#[\PHPUnit\Framework\Attributes\UsesClass(ContextualOptionResolver::class)]
34+
#[\PHPUnit\Framework\Attributes\UsesClass(ProcessHistory::class)]
35+
#[\PHPUnit\Framework\Attributes\UsesClass(ProcessState::class)]
36+
class EventDispatcherTaskTest extends TestCase
37+
{
38+
/** @var list<string> */
39+
private array $calledListeners = [];
40+
41+
/** @var list<string> */
42+
private array $deprecations = [];
43+
44+
public function testEventIsDispatchedUnderEventName(): void
45+
{
46+
$dispatcher = $this->createDispatcher(['myapp.myevent']);
47+
48+
$this->execute($dispatcher, ['event_name' => 'myapp.myevent']);
49+
50+
self::assertSame(['myapp.myevent'], $this->calledListeners);
51+
self::assertSame([], $this->deprecations);
52+
}
53+
54+
public function testEventIsDispatchedUnderClassNameWithoutEventName(): void
55+
{
56+
$dispatcher = $this->createDispatcher(['myapp.myevent', EventDispatcherTaskEvent::class]);
57+
58+
$this->execute($dispatcher, []);
59+
60+
self::assertSame([EventDispatcherTaskEvent::class], $this->calledListeners);
61+
self::assertSame([], $this->deprecations);
62+
}
63+
64+
public function testEventIsDispatchedOnceWhenEventNameIsTheClassName(): void
65+
{
66+
$dispatcher = $this->createDispatcher([EventDispatcherTaskEvent::class]);
67+
68+
$this->execute($dispatcher, ['event_name' => EventDispatcherTaskEvent::class]);
69+
70+
self::assertSame([EventDispatcherTaskEvent::class], $this->calledListeners);
71+
self::assertSame([], $this->deprecations);
72+
}
73+
74+
public function testClassNameListenersAreStillCalledWithDeprecation(): void
75+
{
76+
$dispatcher = $this->createDispatcher(['myapp.myevent', EventDispatcherTaskEvent::class]);
77+
78+
$this->execute($dispatcher, ['event_name' => 'myapp.myevent']);
79+
80+
self::assertSame(['myapp.myevent', EventDispatcherTaskEvent::class], $this->calledListeners);
81+
self::assertCount(1, $this->deprecations);
82+
self::assertStringContainsString('listen to "myapp.myevent" instead', $this->deprecations[0]);
83+
}
84+
85+
public function testPassiveTaskOutputsInput(): void
86+
{
87+
$state = $this->execute($this->createDispatcher([]), [], 'input');
88+
89+
self::assertSame('input', $state->getOutput());
90+
}
91+
92+
public function testInvalidEventNameThrows(): void
93+
{
94+
$this->expectException(InvalidOptionsException::class);
95+
$this->execute($this->createDispatcher([]), ['event_name' => 123]);
96+
}
97+
98+
/**
99+
* @param list<string> $eventNames
100+
*/
101+
private function createDispatcher(array $eventNames): EventDispatcher
102+
{
103+
$dispatcher = new EventDispatcher();
104+
foreach ($eventNames as $eventName) {
105+
$dispatcher->addListener($eventName, function (EventDispatcherTaskEvent $event) use ($eventName): void {
106+
$this->calledListeners[] = $eventName;
107+
});
108+
}
109+
110+
return $dispatcher;
111+
}
112+
113+
private function execute(EventDispatcher $dispatcher, array $options, mixed $input = null): ProcessState
114+
{
115+
$processConfiguration = new ProcessConfiguration('test', []);
116+
$state = new ProcessState($processConfiguration, new ProcessHistory($processConfiguration));
117+
$state->setContextualOptionResolver(new ContextualOptionResolver());
118+
$state->setContext([]);
119+
$state->setTaskConfiguration(new TaskConfiguration('dispatch', EventDispatcherTask::class, $options));
120+
$state->setInput($input);
121+
122+
set_error_handler(function (int $errno, string $errstr): bool {
123+
$this->deprecations[] = $errstr;
124+
125+
return true;
126+
}, \E_USER_DEPRECATED);
127+
try {
128+
$task = new EventDispatcherTask($dispatcher);
129+
$task->initialize($state);
130+
$task->execute($state);
131+
} finally {
132+
restore_error_handler();
133+
}
134+
135+
return $state;
136+
}
137+
}

0 commit comments

Comments
 (0)