symfony: CheckPassportEvent is not an event, it is a command.

Description

Today we figured out, that Symfony\Component\Security\Http\Event\CheckPassportEvent is used as event, but it is obviously a command. Events should only describe what happened in past, not what has to be in the future.

I suggest to have a look at the common Command Pattern. A quick overview is available here. Commands must be like events async dispatchable/executable. However, to keep the implementation simple, it is not necessary to implement the dispatching logic in that way. It is just important that a command handler is not allowed to return something.

Example

An example in PHP is here available.

About this issue

Original URL
State: closed
Created 2 years ago
Comments: 18 (9 by maintainers)

Most upvoted comments

Theory aside, what is the practical limitations of the event and how do you envision the command?

At the current state, this issue seems purely theoritical. For an issue to be actionable, it should be clear what should be changed and - more importantly - what issues it fixes (in order to judge if the costs of change are worth it).

wouterj on May 11, 2022

Theory aside, what is the practical limitations of the event and how do you envision the command?

At the current state, this issue seems purely theoritical. For an issue to be actionable, it should be clear what should be changed and - more importantly - what issues it fixes (in order to judge if the costs of change are worth it).

You are right, I expected the a Symfony dev is able to figure out, how to implement it. Sorry that this was not clear. A good design would be to implement a CommandBussimilar to the EventDispatcher. With a method named execute. For example

public function execute(CommandInterface $command)
{
     // command bus should know which handler is required for which command
     $commandHandler = [COMMAND HANDLER INSTANCE]
     
     // return value has to be ignored
     $commandHandler($command); // invoke for better type safety
}

It is similar to an event dispatcher, but the business use case is another. However, a command can only only have one handler, not like events which can have multiple subscriber or listener.

SoftwareMensch on May 11, 2022

from the current implemenation POV it’s an event, dispatched by sf/event-dispatcher. It’s valid design.

ro0NL on May 11, 2022

i think you are confusing sf/event-dispatcher vs sf/messenger being different patterns.

switching the pattern is not a goal on itself IMHO

ro0NL on May 11, 2022