8000 [Routing] Use env() in route condition by atailouloute · Pull Request #35727 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Routing] Use env() in route condition #35727

New is 8000 sue

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
wants to merge 1 commit into from
Closed

[Routing] Use env() in route condition #35727

wants to merge 1 commit into from

Conversation

atailouloute
Copy link
Contributor
@atailouloute atailouloute commented Feb 14, 2020 •
edited
Loading
Q A
Branch? master
Bug fix? no
New feature? yes
Deprecations? no
Tickets
License MIT
Doc PR TODO

Added an ExpressionLanguageProvider in the Routing component, that provides the env function which could be used in route conditions

An example of the feature

/**
 * @Route("/only-for-dev", condition="env('APP_ENV') === 'dev'")
 */
public function __invoke()
{
   echo "This will be executed only when APP_ENV = dev";
}

With a default value:

/**
 * @Route("/only-for-dev", condition="env('APP_ENV', 'dev') === 'dev'")
 */

@atailouloute atailouloute force-pushed the use-env-in-route-condition branch from d71b869 to bb3ed99 Compare February 14, 2020 17:50
HeahDude
HeahDude reviewed Feb 14, 2020
View reviewed changes
Copy link
Contributor
@HeahDude HeahDude left a comment

Choose a reason for hiding this comment

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

Thanks for working on this

@HeahDude
Copy link
Contributor

Side note: the idea of using the expression language is to have a workaround the current %env()% usage limitation with routes definition.

@atailouloute atailouloute force-pushed the use-env-in-route-condition branch from bb3ed99 to 9961615 Compare February 14, 2020 18:03
@nicolas-grekas nicolas-grekas added this to the next milestone Feb 14, 2020
@atailouloute atailouloute force-pushed the use-env-in-route-condition branch from 9961615 to 440aad7 Compare February 14, 2020 18:16
@atailouloute atailouloute force-pushed the use-env-in-route-condition branch from 440aad7 to b2a5967 Compare February 14, 2020 18:19
@stof
Copy link
Member
stof commented Feb 14, 2020

AFAICT, the service definition is useless for now, as it is not wired in the routing component at all.

@nicolas-grekas
Copy link
Member

I think this doesn't work: reading $_SERVER is not the same as using %env()% (because of env processors + env loaders). People will expect those to be the same.

Instead, I'd suggest to find a way to pre-declare extra key=>value pairs that would be available to conditions (whom values could come from a real %env()% reference from some config file)

We just talked about this on Slack with @HeahDude, I invite you to get in touch with him for details (as he's the one pushing for this to happen :) )

@atailouloute
Copy link
Contributor Author
atailouloute commented Feb 14, 2020 •
edited
Loading

AFAICT, the service definition is useless for now, as it is not wired in the routing component at all.

@stof, It is wired to Routing, there was already a compiler pass that does that
https://github.com/symfony/framework-bundle/blob/master/DependencyInjection/Compiler/AddExpressionLanguageProvidersPass.php#L31

@atailouloute atailouloute mentioned this pull request Feb 17, 2020
Merged
1 task
fabpot added a commit that referenced this pull request Feb 25, 2020
@fabpot
feature #35747 [Routing][FrameworkBundle] Allow using env() in route …
53df70e 
…conditions (atailouloute)

This PR was merged into the 5.1-dev branch.

Discussion
----------

[Routing][FrameworkBundle] Allow using env() in route conditions

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

This is a second implementation of #35727, it overcomes the limitation mentioned by nicolas in (#35727 (comment))

The goal of this feature is to be able to use env variables in Route conditions

```php
/**
 * @route("/only-for-dev", condition="env('APP_ENV') === 'dev'")
 */
public function __invoke()
{
   echo "This will be executed only when APP_ENV = dev";
}
```
it supports also env processors/ loaders
```php
/**
 * @route("/only-for-dev", condition="env('trim:APP_ENV') === 'dev'")
 */
````

**TODOs:**
- [x] Complete unit tests

Commits
-------

b574460 [Routing][FrameworkBundle] Allow using env() in route conditions
@nicolas-grekas nicolas-grekas modified the milestones: next, 5.1 May 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HeahDude HeahDude HeahDude left review comments

Assignees
No one assigned
Labels
Feature Routing Status: Needs Review
Projects
None yet
Milestone
5.1
Development

Successfully merging this pull request may close these issues.
5 participants
@atailouloute @HeahDude @stof @nicolas-grekas @carsonbot
0