diff --git a/CHANGELOG.md b/CHANGELOG.md index 3422c1d4..e918fb94 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ Latest ## Fixes * [#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. * [#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. +* [#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. + +## Deprecated +* [#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. v5.0 ----- diff --git a/docs/04-advanced_workflow.md b/docs/04-advanced_workflow.md index e8b094ca..77354d8c 100644 --- a/docs/04-advanced_workflow.md +++ b/docs/04-advanced_workflow.md @@ -238,9 +238,8 @@ class ProcessFailureListener You can also use the [EventDispatcherTask](reference/tasks/event_dispatcher_task.md) to trigger an event in the middle of a process: it dispatches a `CleverAge\ProcessBundle\Event\EventDispatcherTaskEvent`, giving access to the current -`ProcessState`. Note that in the current implementation the event is dispatched under its class name -(`CleverAge\ProcessBundle\Event\EventDispatcherTaskEvent`): the `event_name` option is required but not passed to the -event dispatcher. +`ProcessState`. The event is dispatched under the `event_name` option or, when it is not set, under its class name +(`CleverAge\ProcessBundle\Event\EventDispatcherTaskEvent`). ## Parallelization diff --git a/docs/reference/tasks/event_dispatcher_task.md b/docs/reference/tasks/event_dispatcher_task.md index 23ac3f98..aedddbbc 100644 --- a/docs/reference/tasks/event_dispatcher_task.md +++ b/docs/reference/tasks/event_dispatcher_task.md @@ -24,10 +24,10 @@ Possible outputs Options ------- -| Code | Type | Required | Default | Description | -|--------------|----------|:--------:|---------|---------------------------------------------------------------| -| `event_name` | `string` | **X** | | Name of the event (see Notes: currently not used to dispatch) | -| `passive` | `bool` | | `true` | If `true`, the input is passed to the output before dispatch | +| Code | Type | Required | Default | Description | +|--------------|----------------|:--------:|---------|------------------------------------------------------------------------------| +| `event_name` | `string\|null` | | `null` | Name of the dispatched event, `null` to use the event class name (see Notes) | +| `passive` | `bool` | | `true` | If `true`, the input is passed to the output before dispatch | Examples -------- @@ -50,6 +50,24 @@ push_data_event: Notes ----- -The event is dispatched without an explicit name (`$eventDispatcher->dispatch($event)`), so its name is the event -class name. Listeners must therefore subscribe to `CleverAge\ProcessBundle\Event\EventDispatcherTaskEvent`; the -`event_name` option is required and validated but not used for dispatching. +The event is dispatched with `$eventDispatcher->dispatch($event, $eventName)`: + +* when `event_name` is set, listeners must subscribe to that name: + +```php +#[AsEventListener(event: 'myapp.data_queue')] +public function onDataQueue(EventDispatcherTaskEvent $event): void +{ + $input = $event->getState()->getInput(); +} +``` + +* when `event_name` is `null`, the event name is the event class name, so listeners must subscribe to + `CleverAge\ProcessBundle\Event\EventDispatcherTaskEvent`. Every `EventDispatcherTask` without `event_name` then + triggers the same listeners. + +**Deprecated since v5, removed in v6.0**: from v4.0 to v5.0, the `event_name` option was ignored and the event was +always dispatched under its class name. To preserve backward compatibility, when `event_name` is set (and differs from the +event class name) and listeners subscribe to `CleverAge\ProcessBundle\Event\EventDispatcherTaskEvent`, the event is also +dispatched under its class name and an `E_USER_DEPRECATED` error is triggered. In v6.0, listen to the configured +`event_name` instead, or remove the `event_name` option. diff --git a/src/Task/Event/EventDispatcherTask.php b/src/Task/Event/EventDispatcherTask.php index fcf8bc92..3f2b8e25 100644 --- a/src/Task/Event/EventDispatcherTask.php +++ b/src/Task/Event/EventDispatcherTask.php @@ -39,14 +39,31 @@ public function execute(ProcessState $state): void $event = new EventDispatcherTaskEvent($state); - $this->eventDispatcher->dispatch($event); + $this->eventDispatcher->dispatch($event, $options['event_name']); + + // @deprecated BC layer since v5, remove me in v6.0: from v4.0 to v5.0, the event was only dispatched under its + // class name, even when event_name was set + if (null !== $options['event_name'] + && EventDispatcherTaskEvent::class !== $options['event_name'] + && $this->eventDispatcher->hasListeners(EventDispatcherTaskEvent::class) + ) { + @trigger_error( + \sprintf( + '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.', + EventDispatcherTaskEvent::class, + $options['event_name'], + ), + \E_USER_DEPRECATED + ); + $this->eventDispatcher->dispatch($event); + } } protected function configureOptions(OptionsResolver $resolver): void { - $resolver->setRequired(['event_name']); + $resolver->setDefault('event_name', null); $resolver->setDefault('passive', true); - $resolver->setAllowedTypes('event_name', ['string']); + $resolver->setAllowedTypes('event_name', ['null', 'string']); $resolver->setAllowedTypes('passive', ['boolean']); } } diff --git a/tests/Task/Event/EventDispatcherTaskTest.php b/tests/Task/Event/EventDispatcherTaskTest.php new file mode 100644 index 00000000..f0882f84 --- /dev/null +++ b/tests/Task/Event/EventDispatcherTaskTest.php @@ -0,0 +1,137 @@ + */ + private array $calledListeners = []; + + /** @var list */ + private array $deprecations = []; + + public function testEventIsDispatchedUnderEventName(): void + { + $dispatcher = $this->createDispatcher(['myapp.myevent']); + + $this->execute($dispatcher, ['event_name' => 'myapp.myevent']); + + self::assertSame(['myapp.myevent'], $this->calledListeners); + self::assertSame([], $this->deprecations); + } + + public function testEventIsDispatchedUnderClassNameWithoutEventName(): void + { + $dispatcher = $this->createDispatcher(['myapp.myevent', EventDispatcherTaskEvent::class]); + + $this->execute($dispatcher, []); + + self::assertSame([EventDispatcherTaskEvent::class], $this->calledListeners); + self::assertSame([], $this->deprecations); + } + + public function testEventIsDispatchedOnceWhenEventNameIsTheClassName(): void + { + $dispatcher = $this->createDispatcher([EventDispatcherTaskEvent::class]); + + $this->execute($dispatcher, ['event_name' => EventDispatcherTaskEvent::class]); + + self::assertSame([EventDispatcherTaskEvent::class], $this->calledListeners); + self::assertSame([], $this->deprecations); + } + + public function testClassNameListenersAreStillCalledWithDeprecation(): void + { + $dispatcher = $this->createDispatcher(['myapp.myevent', EventDispatcherTaskEvent::class]); + + $this->execute($dispatcher, ['event_name' => 'myapp.myevent']); + + self::assertSame(['myapp.myevent', EventDispatcherTaskEvent::class], $this->calledListeners); + self::assertCount(1, $this->deprecations); + self::assertStringContainsString('listen to "myapp.myevent" instead', $this->deprecations[0]); + } + + public function testPassiveTaskOutputsInput(): void + { + $state = $this->execute($this->createDispatcher([]), [], 'input'); + + self::assertSame('input', $state->getOutput()); + } + + public function testInvalidEventNameThrows(): void + { + $this->expectException(InvalidOptionsException::class); + $this->execute($this->createDispatcher([]), ['event_name' => 123]); + } + + /** + * @param list $eventNames + */ + private function createDispatcher(array $eventNames): EventDispatcher + { + $dispatcher = new EventDispatcher(); + foreach ($eventNames as $eventName) { + $dispatcher->addListener($eventName, function (EventDispatcherTaskEvent $event) use ($eventName): void { + $this->calledListeners[] = $eventName; + }); + } + + return $dispatcher; + } + + private function execute(EventDispatcher $dispatcher, array $options, mixed $input = null): ProcessState + { + $processConfiguration = new ProcessConfiguration('test', []); + $state = new ProcessState($processConfiguration, new ProcessHistory($processConfiguration)); + $state->setContextualOptionResolver(new ContextualOptionResolver()); + $state->setContext([]); + $state->setTaskConfiguration(new TaskConfiguration('dispatch', EventDispatcherTask::class, $options)); + $state->setInput($input); + + set_error_handler(function (int $errno, string $errstr): bool { + $this->deprecations[] = $errstr; + + return true; + }, \E_USER_DEPRECATED); + try { + $task = new EventDispatcherTask($dispatcher); + $task->initialize($state); + $task->execute($state); + } finally { + restore_error_handler(); + } + + return $state; + } +}