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

[DIC] Simplify service overloading / specialization #34259

Closed
lyrixx opened this issue Nov 6, 2019 · 7 comments
Closed

[DIC] Simplify service overloading / specialization #34259

lyrixx opened this issue Nov 6, 2019 · 7 comments

Comments

@lyrixx
Copy link
Member

lyrixx commented Nov 6, 2019

Description

In some application, there are many workflows / state machine. And in order to simplify DI, I use the following pattern:

namespace App\Reservations;

use Symfony\Component\Workflow\StateMachine;

class ReservationWorkflow extends StateMachine
{
}

with this configuration:

services:
    App\Reservations\ReservationWorkflow:
        parent: state_machine.reservation

This allow me to "require" a ReservationWorkflow in another service, and I get the right workflow.

(If you wonder, it's not possible to alias ReservationWorkflow to @state_machine.reservation, because the type hint check will fail at run time)

I wondering if we can do something in Symfony to simplify this experience because

  1. writing this code is boring
  2. it's not obvious we can do that
  3. since the service App\Reservations\ReservationWorkflow has a parent, it can not be declared in the main services.yml (_instanceof is not compatible with parent)

Example

It could be nice if we could do that:

services:
    App\Reservations\ReservationWorkflow:
        specialize: state_machine.reservation
        # or any better name

And that all. Symfony will create the class_alias, or generate the code (for autocomplete in IDE)

WDYT ?

@lyrixx lyrixx changed the title [DIC] Simplify service overloading [DIC] Simplify service overloading / specialization Nov 6, 2019
@stof
Copy link
Member

stof commented Nov 6, 2019

Why relying on autowiring a child class rather than relying on named bindings for the autowiring ? Defining an alias Symfony\Component\Workflow\StateMachine $reservationWorkflow: @state_machine.reservation would let you write constructor with arguments StateMachine $reservationWorkflow which would select the right workflow to implement (you could switch that to use the interface in the alias and the typehint btw). And that would not require any weird magic about class aliases (which might not be discovered by your IDE btw, and are tricky to define at the right time).

Btw, FrameworkBundle 4.4 (maybe also 4.3 but I haven't checked) already automatically defines such autowiring typehints for the WorkflowInterface and an argument named with your workflow id ($reservation in your example).

@lyrixx
Copy link
Member Author

lyrixx commented Nov 6, 2019

Why relying on autowiring a child class rather than relying on named bindings for the autowiring

No, I don't want that. I want to rely on the parent class definition. That's why I use the parent key. Did I miss something?

would let you write constructor with arguments StateMachine $reservationWorkflow which would select the right workflow to implement

Symfony could not know, except if you use something like

services:
    _defaults:
        Symfony\Component\Workflow\StateMachine $reservationWorkflow: @state_machine.reservation

Btw, FrameworkBundle 4.4 (maybe also 4.3 but I haven't checked) already automatically defines such autowiring typehints for the WorkflowInterface and an argument named with your workflow id ($reservation in your example).

I forgot about that. Indeed, It could fix indeed my issue. I have to try. But it's a bit magic. As soon as you make a type in the argument name, it does not work anymore. I'm not sure to like this 😬

edit: I check, and It's already availbable in 4.3

bin/console debug:auto
[...]
 Symfony\Component\Workflow\WorkflowInterface $creditNoteStateMachine (state_machine.credit_note)

 Symfony\Component\Workflow\WorkflowInterface $invoiceItemStateMachine (state_machine.invoice_item)

 Symfony\Component\Workflow\WorkflowInterface $invoiceStateMachine (state_machine.invoice)

 Symfony\Component\Workflow\WorkflowInterface $reservationStateMachine (state_machine.reservation)

@nicolas-grekas
Copy link
Member

Named autowiring aliases were created exactly to solve this issue. They need more documentation for sure. The pattern you're using in your description is a hack ;)

@lyrixx
Copy link
Member Author

lyrixx commented Nov 17, 2019

ok, let's close this

@lyrixx lyrixx closed this as completed Nov 17, 2019
@UFTimmy
Copy link

UFTimmy commented Nov 20, 2019

@nicolas-grekas I recently discovered the named auto wire aliases, and they are great.

How do you handle when using the service subscriber interface, though? In my case I am updating older former SF 3 applications to SF 4 and switching to auto wiring. The controllers have a bunch of container calls in them, and the way least prone to breaking things I have found is to drop in the getSubscribedServices method to each controller, so I don't have to change any additional controller code.

How can I specify which workflow I want to get that way, or can I?

I have considered injecting the workflow registry and pulling the workflow I want, but that requires controller code changes, which I would like to avoid, as I am doing this for a bunch of applications.

So is there a general way to use Service Subscribers when there are multiple services that stem from the same class, such as Workflows, EntityManager, Redis caches, etc? The service subscriber requires a FQCN for the key, it does not let you provide a service id.

@lyrixx
Copy link
Member Author

lyrixx commented Nov 20, 2019

I'm reopening this issue because the last concern is valid.

Feel free to re-close it

@lyrixx lyrixx reopened this Nov 20, 2019
@nicolas-grekas
Copy link
Member

@UFTimmy named autowiring aliases are compatible with service subscribers: use the name of the workflow as the key and the type as a value. Done :)

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

5 participants