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.

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

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

Merged
merged 1 commit into from Feb 25, 2020
Merged

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

merged 1 commit into from Feb 25, 2020

Conversation

atailouloute
Copy link
Contributor

@atailouloute atailouloute commented Feb 17, 2020 •
edited

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 (it supports also env processors/ loaders)

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

TODOs:

  • Complete unit tests

@atailouloute
Copy link
Contributor Author

/cc @nicolas-grekas @HeahDude Here is an experimentation/second implementation of #35727

@nicolas-grekas nicolas-grekas added this to the next milestone Feb 17, 2020
@atailouloute atailouloute force-pushed the use-env-in-route-condition branch 2 times, most recently from e1f164c to 84a22c6 Compare February 17, 2020 14:32
@atailouloute atailouloute changed the title [Routing][DependencyInjection][FrameworkBundle] Use env() in route condition [Routing][FrameworkBundle] Use env() in route condition Feb 17, 2020
@atailouloute atailouloute force-pushed the use-env-in-route-condition branch 3 times, most recently from b14fb2e to 2d30cf3 Compare February 17, 2020 15:59
@atailouloute atailouloute force-pushed the use-env-in-route-condition branch 3 times, most recently from 732ef18 to a40e319 Compare February 17, 2020 16:51
@atailouloute atailouloute force-pushed the use-env-in-route-condition branch 5 times, most recently from 6177069 to 9431dc6 Compare February 17, 2020 20:53
nicolas-grekas
nicolas-grekas approved these changes Feb 17, 2020
View reviewed changes
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

Great, I like it a lot :)
Some minor details and good to go on my side.

src/Symfony/Component/Routing/Tests/Matcher/ExpressionLanguageTest.php Outdated Show resolved Hide resolved
src/Symfony/Component/Routing/Tests/Matcher/ExpressionLanguageTest.php Outdated Show resolved Hide resolved
HeahDude
HeahDude approved these changes Feb 17, 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.

Looks good, thanks @atailouloute @nicolas-grekas

maxhelias
maxhelias approved these changes Feb 18, 2020
View reviewed changes
Copy link
Contributor

@maxhelias maxhelias left a comment

Choose a reason for hiding this comment

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

Nice!

@nicolas-grekas nicolas-grekas changed the title [Routing][FrameworkBundle] Use env() in route condition [Routing][FrameworkBundle] Allow using env() in route condition Feb 18, 2020
@nicolas-grekas nicolas-grekas changed the title [Routing][FrameworkBundle] Allow using env() in route condition [Routing][FrameworkBundle] Allow using env() in route conditions Feb 24, 2020
@nicolas-grekas nicolas-grekas force-pushed the use-env-in-route-condition branch 3 times, most recently from ff17ac7 to 357fc77 Compare February 24, 2020 11:21
nicolas-grekas
nicolas-grekas approved these changes Feb 24, 2020
View reviewed changes
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

FYI, I improved the feature by adding a new routing.expression_language_function tag to make adding new functions easy. This leverages ServiceProviderInterface and tagged locators.

@fabpot
Copy link
Member

fabpot commented Feb 25, 2020

Thank you @atailouloute.

@fabpot fabpot mentioned this pull request Feb 25, 2020
Closed
@fabpot fabpot merged commit 53df70e into symfony:master Feb 25, 2020
@atailouloute atailouloute deleted the use-env-in-route-condition branch April 2, 2020 08:34
@nicolas-grekas nicolas-grekas modified the milestones: next, 5.1 May 4, 2020
@fabpot fabpot mentioned this pull request May 5, 2020
Merged
@GromNaN GromNaN mentioned this pull request Mar 16, 2022
Merged
@GromNaN
Copy link
Member

GromNaN commented Mar 16, 2022

Thanks for this feature @atailouloute. I used it today to match host using env var; only way to make it dynamic with prebuild Symfony cache.
Doc PR symfony/symfony-docs#16610

javiereguiluz added a commit to symfony/symfony-docs that referenced this pull request Mar 25, 2022
@javiereguiluz
minor #16610 [Routing] doc for env() function in route condition (Gro…
2df2303 
…mNaN)

This PR was merged into the 5.4 branch.

Discussion
----------

[Routing] doc for env() function in route condition

Fix #13247 for symfony/symfony#35747

Very useful to match host based on env var.

```yaml
blog_list:
    path: /blog
    # the controller value has the format 'controller_class::method_name'
    controller: App\Controller\BlogController::list
    condition: "context.getHost() == env('APP_MAIN_HOST')"
```

Commits
-------

c22dc20 Documentation for env() function in route condition
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot approved these changes

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

@chalasr chalasr chalasr approved these changes

@HeahDude HeahDude HeahDude approved these changes

@maxhelias maxhelias maxhelias approved these changes

@Tobion Tobion Awaiting requested review from Tobion

Assignees
No one assigned
Labels
DependencyInjection Feature FrameworkBundle Routing Status: Reviewed
Projects
None yet
Milestone
5.1
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@atailouloute @fabpot @GromNaN @nicolas-grekas @Tobion @chalasr @HeahDude @maxhelias @carsonbot