#189 - Fix EventDispatcherTask ignoring the event_name option

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
 -----

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
 

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.

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']);
     }
 }

tests/Task/Event/EventDispatcherTaskTest.php

@@ -0,0 +1,137 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the CleverAge/ProcessBundle package.
+ *
+ * Copyright (c) Clever-Age
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace CleverAge\ProcessBundle\Tests\Task\Event;
+
+use CleverAge\ProcessBundle\Configuration\ProcessConfiguration;
+use CleverAge\ProcessBundle\Configuration\TaskConfiguration;
+use CleverAge\ProcessBundle\Context\ContextualOptionResolver;
+use CleverAge\ProcessBundle\Event\EventDispatcherTaskEvent;
+use CleverAge\ProcessBundle\Model\AbstractConfigurableTask;
+use CleverAge\ProcessBundle\Model\ProcessHistory;
+use CleverAge\ProcessBundle\Model\ProcessState;
+use CleverAge\ProcessBundle\Task\Event\EventDispatcherTask;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\EventDispatcher\EventDispatcher;
+use Symfony\Component\OptionsResolver\Exception\InvalidOptionsException;
+
+#[\PHPUnit\Framework\Attributes\CoversClass(EventDispatcherTask::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(EventDispatcherTaskEvent::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(AbstractConfigurableTask::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(ProcessConfiguration::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(TaskConfiguration::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(ContextualOptionResolver::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(ProcessHistory::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(ProcessState::class)]
+class EventDispatcherTaskTest extends TestCase
+{
+    /** @var list<string> */
+    private array $calledListeners = [];
+
+    /** @var list<string> */
+    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<string> $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;
+    }
+}