Hey!

To help keep things organized, we don't allow "Draft" pull requests. Could you please click the "ready for review" button or close this PR and open a new one when you are done?

Note that a pull request does not have to be "perfect" or "ready for merge" when you first open it. We just want it to be ready for a first review.

Cheers!

Carsonbot

On second though, we might want to go a bit further: with the current approach, referenced services are going to be instantiated all the time, while they might be needed only on very specific routes.
What about providing a service locator instead, after making EL able to deal with them?

What if two services are registered with the same name? This could cause some confusing issues, every service will override the one registered before.

If docs will be added: prefix the service names in third party bundles.

@nicolas-grekas

On second though, we might want to go a bit further: with the current approach, referenced services are going to be instantiated all the time, while they might be needed only on very specific routes.
What about providing a service locator instead, after making EL able to deal with them?

Instantiating services all the time is indeed not good. Based on your feedback I was wondering if we could inject a function that relies on a service locator instead of injecting variables

#[Route('/page', condition: "service('myService').myMethod()")]

Pros:

• No changes in UrlMatcher (we can use the already available function provider feature)
• We'll create services only when requested

What do you think?

I like it @renanbr!

Status: Since eafc9c5, the FrameworkBundle wraps tagged services into a ServiceLocator, then passes it to the routing component, which unpacks services in the matcher when necessary.

I'm satisfied with this ☝🏼 solution.

My next shot will be implementing service('name') as described in #44405 (comment)

Why not using the ExpressionFunctionProviderInterface extension point instead of creating a new way of extending the expression language ?

Why not using the ExpressionFunctionProviderInterface extension point instead of creating a new way of extending the expression language ?

For convenience I think. What would be the DX of your proposal?

@renanbr up to finish this PR?

Why not using the ExpressionFunctionProviderInterface extension point instead of creating a new way of extending the expression language ?

For convenience I think. What would be the DX of your proposal?

Late it the discussion. A new function service() would provide access to the container.

class DefaultController
{
    #[Route('/page', condition: 'service('foo').bar()')]
    public function page(): Response { ... }
} 

A new function service() would provide access to the container.

The one true container you mean? I guess that's no-go since that'd require services to be made public. The benefit of this proposal is that it allows labeling which services need to be made available for the routing layer.

Maybe a service locator, feed with regular tagged services.

That's exactly what this PR does IIUC :)

Indeed. Then the description needs to be updated.

@renanbr up to finish this PR?

I'll work on this week and the next week.

thanks for the review btw

updated,
latest changes

• rename tag to routing.condition_service;
• rename tag identifier to alias;
• rename class attribute to AsRoutingConditionService;
• move class attribute under FrameworkBundle;
• rename expression language function to container.get_routing_condition_service;
• add tests

Thanks @renanbr for working on this feature, this is much appreciated.