8000 Event bus by mvrhov · Pull Request #4 · sroze/symfony · GitHub
[go: up one dir, main page]

Skip to content

Event bus #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related e 8000 mails.

Already on GitHub? Sign in to your account

Open
wants to merge 16 commits into
base: add-message-component
Choose a base branch
from
Open

Event bus #4

wants to merge 16 commits into from

Conversation

mvrhov
Copy link
@mvrhov mvrhov commented Dec 26, 2017

This adds the Event bus along with the Message bus.

The example in current documentation, with email sending is bad(TM) so we need to find a better one.
Imagine that we have an middleware that wraps command handling into an transaction. This means that the commit will happen after the command has been handled, and if there is something wrong during the commit, we could have easily already sent the email however the user wasn't persisted.
What should instead happen 8000 is that message handler that created an user should instead record an event/new message at the end of the __invoke function.
Then there should be middleware that's executed last which gets the recorded events and publishes them to another bus named Event bus.

example bellow:

namespace Component\User\Model\Event;

use Component\User\Model\User;

class UserRegistered
{
    /** @var User */
    private $user;

    public function __construct(User $user)
    {
        $this->user = $user;
    }

    public function user(): User
    {
        return $this->user;
    }
}
namespace Component\User\Model\Handler;

use Component\Security\PasswordEncoder\PasswordEncoderInterface;
use Component\User\Model\Event\UserRegistered;
use Component\User\Model\User;
use Component\User\Model\UserRepository;
use Symfony\Component\Message\Recorder\RecordsMessages;

class RegisterUser
{
    /** @var UserRepository */
    private $repository;
    /** @var RecordsMessages */
    private $eventRecorder;
    /** @var PasswordEncoderInterface */
    private $encoder;

    public function __construct(UserRepository $repository, RecordsMessages $eventRecorder, PasswordEncoderInterface $encoder) {
        $this->repository = $repository;
        $this->eventRecorder = $eventRecorder;
        $this->encoder = $encoder;
    }

    public function __invoke(\Component\User\Model\Command\RegisterUser $cmd)
    {
        $user = User::registerWith($cmd->getId(), $cmd->getEmail());

        $user->changePassword($this->encoder->encodePassword($cmd->getPassword()));
        $user->enable();
        if ($cmd->isSuperAdmin()) {
            $user->promoteToSuperAdmin();
        }
        $user->changeFirstName($cmd->getFirstName());

        $this->repository->save($user);

        $event = new UserRegistered($user);
        $this->eventRecorder->record($event);
    }
}
namespace Component\User\Infra\EventHandler;

use Component\User\Model\Event\UserRegistered;

class RegisteredUserEmailSender
{
    private $emailSender;
    private $mailer;
    
    public function __construct(\Swift_Mailer $mailer, string $emailSender)
    {
        $this->mailer = $mailer;
        $this->emailSender = $emailSender;
    }
    
    public function __invoke(UserRegistered $user)
    {        
        $this->mailer->send(
            (new \Swift_Message('Welcome to the site'))
                ->setTo($user->user()->email())
                ->setSender($this->emailSender)
                ->setBody(
                    '<h1>Hello '.$user->user()->firstName().'</h1><p>Welcome to the site</p>',
                    'text/html'
                )
        );
    }
}

the recorder is taken directly from SimpleBus, that's the reason behind the author tag. I hadn't done any renaming as there will probably be a lot of discussion here.
@sroze will you merge this so the discussion happens in the main Symfony repo?

@sroze
Copy link
Owner
sroze commented Dec 27, 2017

That's a very interesting question here.

As of now, I don't think that we need such "event bus" in the "default package". All you need - as a user - is to define multiple buses if you'd like to (maybe we can use some configuration in the FrameworkBundle). Therefore, I'll mention this PR in the Symfony PR :)

@mvrhov
Copy link
Author
mvrhov commented Dec 27, 2017

The problem is that just multiple buses is not enough. You really need to store the events somewhere and dispatch them after the current bus is done with them. You don't know what other middlewares will do.

@sroze
Copy link
Owner
sroze commented Dec 27, 2017

Oh yeah, and the "store middleware" you've added :)

@mvrhov
Copy link
Author
mvrhov commented Dec 27, 2017

Deleted. Will move this into the symfony thread

@sroze sroze force-pushed the add-message-component branch from 1c81979 to 56d5da4 Compare January 26, 2018 13:37
sroze pushed a commit that referenced this pull request Feb 3, 2018
This PR was merged into the 3.3 branch.

Discussion
----------

Restore RoleInterface import

| Q             | A
| ------------- | ---
| Branch?       | 3.3
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no <!-- don't forget to update UPGRADE-*.md files -->
| Tests pass?   | yes
| License       | MIT

The import is use on PHPDoc but was accidentally removed. Maybe because PHPStorm does not match with the import when you use parenthesis.

Not really a bug as it is concerning only PHPDoc, but it make some analysis tools like PHPStan yelling:

```
 ------ -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  Line   src/AppBundle/Security/Authentication/ApiKeyAuthenticator.php
 ------ -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  64     Parameter #4 $roles of class Symfony\Component\Security\Core\Authentication\Token\PreAuthenticatedToken constructor expects array<string|Symfony\Component\Security\Core\Authentication\Token\RoleInterface>, array<string|Symfony\Component\Security\Core\Role\Role>
         given.
 ------ -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

 ------ -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  Line   tests/AppBundle/Controller/WebTestCase.php
 ------ -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  59     Parameter #4 $roles of class Symfony\Component\Security\Core\Authentication\Token\PreAuthenticatedToken constructor expects array<string|Symfony\Component\Security\Core\Authentication\Token\RoleInterface>, array<string|Symfony\Component\Security\Core\Role\Role>
         given.
 ------ -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
```

Commits
-------

8ecfeb1 Restore RoleInterface import
@sroze sroze force-pushed the add-message-component branch 3 times, most recently from 3967eda to e63c5b9 Compare March 19, 2018 17:54
@sroze sroze force-pushed the add-message-component branch from 86a91ec to c091d66 Compare March 21, 2018 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0