Skip to content
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 emails.

Already on GitHub? Sign in to your account

RFC: Autowiring commands to handlers in Symfony 3.3+ #44

Closed
rosstuck opened this issue May 28, 2017 · 9 comments
Closed

RFC: Autowiring commands to handlers in Symfony 3.3+ #44

rosstuck opened this issue May 28, 2017 · 9 comments

Comments

@rosstuck
Copy link
Member

Based on the new service discovery coming in Symfony 3.3, I'd like to add a naming convention based locator.

The idea is that we could use config like:

services:
    App\:
        resource: '../../src/{Handler}'
        tags: ['tactician.handler']

This would autowire all handlers into the DI container using their FQCN as service id (RegisterUserHandler).

The proposed new locator would automatically route a command with a FQCN like Foo\RegisterUser or Foo\RegisterUserCommand to Foo\RegisterUserHandler. We could pull that directly from the container/service locator and presto, it works.

Pros:

  • The user has all of their correct mappings in 3 lines of config, much less config typing, especially since IDEs don't autocomplete the command class name in services.yml
  • No more worrying about updating the command names in the services.yml after doing a rename on a command.
  • Much simpler onboarding for those using autowiring.

Cons:

  • Much magic, such auto, very wow.
  • We definitely need to add a switch to make this optional.
  • Mainly works in Symfony 3.3 and up.

Thoughts:

  • We could add a naming inflector interface for this autowiring so folks can add in their own class name conventions.

While I'm only so-so on autowiring containers and generally avoid it, I think this is a feature that pro-autowiring folks would really enjoy.

Thoughts? Totally open to all config format/feature/implementation suggestions here. :)

@ogizanagi
Copy link
Contributor

ogizanagi commented May 28, 2017

It'd be great if it's also flexible enough & configurable out of the box.
In my own projects, I declare commands, queries and handlers in dedicated Command, Query, Handler namespaces (it might seem redundant, but it helps well to keep my Application layer structured and not overwhelming).

E.g: Foo\Application\User\Command\RegisterUserCommand should be handled by Foo\Application\User\Handler\RegisterUserCommandHandler

@tyx
Copy link
Contributor

tyx commented May 29, 2017

IMO everyone have its own habits regarding stuff naming. As @ogizanagi said we need this very flexible to be used.

In our apps, we have not dedicated handlers for example, so it becomes difficult to make magic I guess ;)

@rosstuck rosstuck mentioned this issue Jun 2, 2017
17 tasks
@ogizanagi
Copy link
Contributor

Note that in the yaml configuration shown in the description:

services:
    App\:
        resource: '../../src/{Handler}'
        tags: ['tactician.handler']

the tag could also be completely removed if we introduce a marker interface, thanks to the autoconfigure capabilities of sf 3.3.
Above all, it also means you don't to write anything specific to handlers in the DI configuration, but simply keep the defaults proposed by the framework recipe using flex to PSR-4 load the whole src/ directory, and have everything configured & autowired automatically.

A similar marker is under discussion for Twig runtimes, which currently don't have any interface either.

@mateuszsip
Copy link
Contributor

This is the tough one.

In my project we use notation like this:
Foo\Application\User\RegisterUser\RegisterUser for command and:
Foo\Application\User\RegisterUser\RegisterUserHandler for command handler.
Plus marker interfaces.

We could add a naming inflector interface for this autowiring so folks can add in their own class name conventions.

Sounds great, with a few most popular implementations it can work really well.

If you need any help, I would be glad to.

@jvasseur
Copy link

jvasseur commented Jun 3, 2017

If we add an handler interface maybe we could add a static method to it that returns the corresponding command to remove the need for an inflector.

@rosstuck
Copy link
Member Author

rosstuck commented Jun 3, 2017 via email

@ogizanagi
Copy link
Contributor

ogizanagi commented Jun 3, 2017

I think it's slightly different here. No need to be in the core. It'll only serve autoconfiguring purpose, which is a symfony thing, thus should only be in the bundle. This marker interface should not serve any other means.

@sgomez
Copy link

sgomez commented Jun 9, 2017

Besides, using a marker interface could be the best way to differentiate who wants to use autowiring or not.

@rosstuck
Copy link
Member Author

Closing in favor of #60 and the PR #67.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants