10000 [Messenger] Allow different types of busses · Issue #26652 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Messenger] Allow different types of busses #26652

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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom
Closed
Nyholm opened this issue Mar 23, 2018 · 8 comments
Closed

[Messenger] Allow different types of busses #26652

Nyholm opened this issue Mar 23, 2018 · 8 comments
Labels
Feature Messenger

Comments

@Nyholm
Copy link
Member
Nyholm commented Mar 23, 2018 •
edited
Loading
Q A
Bug report? no
Feature request? yes
BC Break report? no
RFC? yes
Symfony version 4.1

I want to have multiple buses with different features. Example of different buses might be:

Default bus

  • Allows zero or more handlers
  • Supports both sync and async
  • Return values are optional

Command bus

  • Exactly one handler required
  • Supports both sync and async
  • No return values from handlers

Event bus

  • Allows zero or more handlers
  • Supports both sync and async
  • No return values from handlers

Query bus

  • Exactly one handler required
  • Supports only sync
  • Return values required

Example config:

framework:
  messenger:
    middlewares: ~ # default middlewares
    bus_foo:
      type: foo
      routing:
        'App\Bar': ['sender.bar', 'sender.biz']
        'App\Foo': 'sender.foo'
      middlewares:  # bus specific middlewares
        - 'custom_service'
        - validation: true
@mvrhov
Copy link
mvrhov commented Mar 23, 2018

The Whole Event bus with recorder taken from Simple bus package PR is still on sroze's fork of symfony

@sroze
Copy link
Contributor
sroze commented Mar 24, 2018

I think here there are two topics here.

  1. Letting FrameworkBundle create multiple buses
    It might be a good idea, but obviously increases the complexity. I have no idea whether it will be a common need (i.e. more than the "20%") or not. Therefore, before deciding anything like that, I'd argue we should wait a bit :)

  2. Apply certain rules for each of these buses
    I think that this should typically live outside of Symfony Core for the time being. These topics are opinionated and most will have different opinions depending on the use-case. IMHO, a very good contribution would be a library providing a set of middlewares enforcing these rules. I'm planning to work on these as well but if you want to get started before, that would be 👌

@chalasr chalasr mentioned this issue Mar 27, 2018
Closed
@Nyholm Nyholm mentioned this issue Mar 27, 2018
Merged
@sroze sroze mentioned this issue Mar 29, 2018
Merged
1 task
@romaricdrigon
Copy link
Contributor

Hello,
Being able to easily declare multiple buses would be great (at least having some documented YAML configuration you can add). It is pretty much required for using the Messenger component as a command bus. Plus I see having middleware declared on all buses as possibly surprising and causing issues.
An approach like Tactician would be great: having some sane defaults, easy to get started, and being able to override and configure when you want to go further.

@sroze
Copy link
Contributor
sroze commented Apr 5, 2018

As a first step, I suggest we allow to define multiple buses like that:

framework:
    messenger:
        buses:
            default: ~
            command: ~
            query: ~

  1. Each of these bus configurations will a messenger.bus.$name service. If there is a bus named default then we create an alias message_bus (as it is right now).

  2. We keep defining the middlewares via tags (for now at least). If the middleware tag (to be renamed messenger.bus_middleware IMHO) has a bus attribute (names of the buses, comma separated) it is only configured for those buses. If no, then it's applied for all the buses.

We would have the "calling handler" and "routing the sender" middlewares tagged without a given bus so they would be shared across all the buses.

@mvrhov
Copy link
mvrhov commented Apr 5, 2018 •
edited
Loading

@sroze: This is not enough. e.g for event bus you need a middleware registered, that won't throw if there is no handler for a specific "command"

edit: It seems that I missed the 2nd part of your post, where you say, that the tag will have the bus attribute.

@romaricdrigon
Copy link
Contributor

That seems to be a robust start 👍

Later, what about adding middleware in the YAML configuration, instead of using tags?
I may not be up-to-date with the "latest Symfony style", but it looks clearer to me.
In example:

framework:
    messenger:
        buses:
             all:
                 middlewares:
                  - "Symfony\\Messenger\\HandlerMiddleware"
            default: ~
            command:
                middlewares:
                    - "App\\MyMiddleware"

Another option would be a to use a Doctrine-bundle style configuration, though I find it more ambiguous (are middlewares on the top overriden? Then we should re-add them?...).

framework:
    messenger:
        middlewares: # Will apply to all, Doctrine-bundle configuration style
            - "Symfony\\Messenger\\HandlerMiddleware"
        buses:
            default: ~
            command:
                 middlewares:
                     - "Symfony\\Messenger\\HandlerMiddleware"
                     - "App\\MyMiddleware"

@ro0NL
Copy link
Contributor
ro0NL commented Apr 8, 2018 •
edited
Loading

I like the idea of a default bus (one that can be autowired) and knows about all configured handlers, with a "zero or more handlers" requirement.

8000

@sroze sroze mentioned this issue Apr 8, 2018
Merged
@sroze
Copy link
Contributor
sroze commented Apr 8, 2018

Pull-request is here: #26864 🚀

sroze added a commit that referenced this issue Apr 25, 2018
@sroze
feature #26864 [Messenger] Define multiple buses from the `framework.…
da4fccd 
…messenger.buses` configuration (sroze)

This PR was squashed before being merged into the 4.1-dev branch (closes #26864).

Discussion
----------

[Messenger] Define multiple buses from the `framework.messenger.buses` configuration

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #26652
| License       | MIT
| Doc PR        | symfony/symfony-docs#9617

Not everybody will benefit from having only one bus, especially with the CQRS-like usages. While keeping the extremely use of use of the single bus, this PR has the following:

- Create multiple buses from the YAML configuration
- Tag middleware only a specific buses
- Register middlewares from the YAML configuration

Even if it's visible in the PR's tests, here's how it will look like, for a completely full-customised version:
```yaml
framework:
    messenger:
        default_bus: commands
        buses:
            commands: ~
            events:
                middlewares:
                    - validation
                    - route_messages
                    - "Your\\Middleware\\Service"
                    - call_message_handler
```

A few things to note:
1. The YAML configuration creates `messenger.bus.[name]` services for the buses.
2. The YAML configuration for middleware just adds tags to the corresponding middlewares.
3. If the middleware definition does not exists, it creates it. (without any magic on the arguments though, if it isn't auto-wirable, well... "your problem")
4. In the PR, there is this "TolerateNoHandler" middleware that is a great example for event buses

Commits
-------

e5deb84 [Messenger] Define multiple buses from the `framework.messenger.buses` configuration
@sroze sroze closed this as completed Apr 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Feature Messenger
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@mvrhov @sroze @romaricdrigon @ro0NL @Nyholm @xabbuh
0