Skip to content

[DependencyInjection] Add support for autowiring services as closures using attributes #49628

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
Merged
merged 1 commit into from
Mar 9, 2023
Merged

[DependencyInjection] Add support for autowiring services as closures using attributes #49628

merged 1 commit into from
Mar 9, 2023

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Mar 7, 2023 •
edited
Loading

Q A
Branch? 6.3
Bug fix? no
New feature? yes
Deprecations? no
Tickets -
License MIT
Doc PR -

When dealing with laziness on the consumer side, a common pattern is to wrap a heavy service in a closure and call it only when needed.

This PR adds attribute #[AutowireServiceClosure] for this purpose:

// generate a closure that returns service "foo"
public function __construct(
    #[AutowireServiceClosure('foo')]
    \Closure $foo,
)

It also adds support for turning callables into closures with a new #[AutowireCallable] attribute:

// generate a closure that calls "foo"::someMethod()
public function __construct(
    #[AutowireCallable(service: 'foo', method: 'someMethod')]
    \Closure $foo,
)

Of course, this fully leverages autowiring aliases, so that instead of "foo", you can use MyInterface::class.

@nicolas-grekas nicolas-grekas requested a review from dunglas as a code owner March 7, 2023 10:55
@carsonbot carsonbot added this to the 6.3 milestone Mar 7, 2023
@nicolas-grekas nicolas-grekas force-pushed the di-autowire-closure branch 2 times, most recently from 377e267 to dc3f5e7 Compare March 7, 2023 16:16
@Neirda24
Copy link
Contributor

Neirda24 commented Mar 7, 2023

Would this also be supported through yaml auto wiring ?

@GromNaN

This comment was marked as resolved.

Sign in to view
@derrabus

This comment was marked as resolved.

Sign in to view
@nicolas-grekas nicolas-grekas changed the title [DependencyInjection] Add support for autowiring services as closures when using #[Autowire] [DependencyInjection] Add support for autowiring services as closures using attributes Mar 8, 2023
@nicolas-grekas
Copy link
Member Author

Thanks for the feedback. PR (and description) updated with two new attributes: #[AutowireCallable] and #[AutowireServiceClosure].

@nicolas-grekas nicolas-grekas mentioned this pull request Mar 8, 2023
Merged
@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented Mar 8, 2023 •
edited
Loading

Would this also be supported through yaml auto wiring ?

@Neirda24 this is already supported, see eg #41176 and #45878 which introduced !service_closure and !closure YAML tags respectively.

GromNaN
GromNaN approved these changes Mar 8, 2023
View reviewed changes
Copy link
Member

@GromNaN GromNaN left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is much more expressive and self-documented.

@Neirda24
Copy link
Contributor

Neirda24 commented Mar 8, 2023

Would this also be supported through yaml auto wiring ?

@Neirda24 this is already supported, see eg #41176 and #45878 which introduced !service_closure and !closure YAML tags respectively.

Missed it ! Thanks 👍

@nicolas-grekas
Copy link
Member Author

Merging as this blocks other PRs right now, but feel free to review after merge of course!

@nicolas-grekas nicolas-grekas merged commit 2f1ccef into symfony:6.3 Mar 9, 2023
@nicolas-grekas nicolas-grekas mentioned this pull request Mar 9, 2023
Closed
@nicolas-grekas nicolas-grekas deleted the di-autowire-closure branch March 9, 2023 16:09
@nicolas-grekas nicolas-grekas mentioned this pull request Mar 9, 2023
Merged
nicolas-grekas added a commit that referenced this pull request Mar 10, 2023
@nicolas-grekas
minor #49500 [HttpKernel] Add tests for #[TaggedIterator] & `#[Tagg…
844df3e 
…edLocator]` on controller arguments (HypeMC)

This PR was merged into the 6.3 branch.

Discussion
----------

[HttpKernel] Add tests for `#[TaggedIterator]` & `#[TaggedLocator]` on controller arguments

| Q             | A
| ------------- | ---
| Branch?       | 6.3
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #49083
| License       | MIT
| Doc PR        | -

~~I think this doesn't qualify as a bug fix, but an improvement. If I'm wrong please let me know.~~

#49628 fixed the issue, this PR only adds tests now.

Commits
-------

121e072 [DependencyInjection] Add tests for #[TaggedIterator] & #[TaggedLocator] on controller arguments
fabpot added a commit that referenced this pull request Mar 10, 2023
@fabpot
feature #49642 [DependencyInjection] Deprecate #[MapDecorated] in f…
a7e0b05 
…avor of `#[AutowireDecorated]` (nicolas-grekas)

This PR was merged into the 6.3 branch.

Discussion
----------

[DependencyInjection] Deprecate `#[MapDecorated]` in favor of `#[AutowireDecorated]`

| Q             | A
| ------------- | ---
| Branch?       | 6.3
| Bug fix?      | no
| New feature?  | no
| Deprecations? | yes
| Tickets       | -
| License       | MIT
| Doc PR        | -

`#[Map*]` are for controller argument resolvers.

`#[Autowire*]` should be used for autowiring (see #49628).

We could also rename `#[TaggedLocator]` and `#[TaggedIterator]` but I feel like the need is less obvious and the cost more important.

Commits
-------

b653adf [DependencyInjection] Deprecate `#[MapDecorated]` in favor of `#[AutowireDecorated]`
@fabpot fabpot mentioned this pull request May 1, 2023
Merged
@ruudk ruudk mentioned this pull request Jun 12, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GromNaN GromNaN GromNaN approved these changes

@dunglas dunglas Awaiting requested review from dunglas dunglas is a code owner

Assignees
No one assigned
Labels
DependencyInjection Feature Status: Reviewed
Projects
None yet
Milestone
6.3
Development

Successfully merging this pull request may close these issues.
5 participants
@nicolas-grekas @Neirda24 @GromNaN @derrabus @carsonbot