Skip to content

[RFC] Exclude patterns of controllers from being imported #31516

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
Closed
tristanbes opened this issue May 16, 2019 · 9 comments
Closed

[RFC] Exclude patterns of controllers from being imported #31516

tristanbes opened this issue May 16, 2019 · 9 comments
Labels
Feature Routing

Comments

@tristanbes
Copy link
Contributor

tristanbes commented May 16, 2019 •
edited
Loading

Description
Right now, loading actions or controllers on debug mode only is very messy. A lot of solutions exists, but none of them are, in my point of view, "clean".

Example
Let's say you want to create a Controller with actions that will allow your team to access your app emails in the browser.

You can easily do this with 2 proposition.

Proposition A

Already fixed/implemented in #30379 as pointed by @ro0NL

Allow expression language to use app variable inside the condition key

Right now only context and request can be used.

<?php 

namespace App\Controller;

class DebugEmailController extends AbstractController
{
    /**
     * @Route("/debug/email/registration", condition="%kernel.debug%")
     */
    public function registrationEmail()
    {
    }
}

Proposition B

Like resource loading for service, offer a way to exclude/ignore a pattern of Controllers like:

// config/routes/annotations.yaml
controllers:
    resource: ../../src/Controller/
    type: annotation
    exclude: '../src/Controller/{DebugEmailController}.php'
// config/routes/dev/annotations.yaml
controllers:
    resource: ../../src/Controller/DebugEmailController.php
    type: annotation

Existing and not fine solutions for this case are:

Write the routes of this controller in `yml` and import them only in `dev/routing.yaml`

You are forcing the developper to switch formats which is bad. If they choose annotations for their entire application/company, then you would have 1% of route defined as yaml as an exception just because you can't do it with sticking with annotation format.

You would never recommend to write service registration in .yaml and then, because you want to do something different, adivse them to write them in .xml format.

Write code inside each action of your controller to return a 404 if `app.debug === true`

The cons of this: That would work, but Symfony would still load this routes (also, code duplication).

Put the `DebugEmailController` outside `src/Controller/`

Same remark than before, you would have 99% of your Controller under the "standardized" folder, but then 1% inside a different place to allow to load it only in debug mode ?

Create a bundle and require it only in dev-deps with composer

Well that works well but in this use case, some of the code relies in your application, so it would mean that the bundle will search for templates that lives under /templates. It also create unecessary complexity to work with.

Is there any technical/functional reasons not to have already one of those 2 solution implemented that i'm not aware of ?
@tristanbes tristanbes changed the title [RFC] Exclude some controllers from being imported OR allow app usage in expression language on route loading [RFC] Exclude patterns of controllers from being imported AND/OR allow app. usage in expression language on route loading May 16, 2019
@ro0NL
Copy link
Contributor

ro0NL commented May 16, 2019

proposition A is the same as #30366 right? (fixed in #30379)

@tristanbes
Copy link
Contributor Author

Mhhh, that's really great news, indeed @ro0NL. I'll rename the title of the issue.

@tristanbes tristanbes changed the title [RFC] Exclude patterns of controllers from being imported AND/OR allow app. usage in expression language on route loading [RFC] Exclude patterns of controllers from being imported May 17, 2019
@vudaltsov
Copy link
Contributor

Nice proposal. Another use case is when you want to add smth like prefix: { ru: /, en: /en } for all routes except Api/*.

@nicolas-grekas
Copy link
Member

PR welcome I guess :)

@nicolas-grekas
Copy link
Member

Note that we should allow exclude to be an array of patterns too, as allowed for services.

@vudaltsov
Copy link
Contributor

@tristanbes, do you want to implement this? If you don't, I would like to work on the issue. Since it's yours, you have a full right to do a PR.

@Kocal
Copy link
Member

Kocal commented May 22, 2019 •
edited
Loading

I'm sure @tristanbes would be happy to work on it :) 🚎

@tristanbes
Copy link
Contributor Author

@vudaltsov , I'll give it a try, it could be an awesome feature for a first code contribution.

If I fail I will pass my PR along to you in order not to block this feature from being implemented for a too long time.

@vudaltsov
Copy link
Contributor

Cool, you can ask me on Slack if you need any help :)

@tristanbes tristanbes mentioned this issue May 22, 2019
Merged
nicolas-grekas added a commit that referenced this issue Nov 4, 2019
@nicolas-grekas
feature #31587 [Routing][Config] Allow patterns of resources to be ex…
13dd18c 
…cluded from config loading (tristanbes)

This PR was merged into the 4.4 branch.

Discussion
----------

[Routing][Config] Allow patterns of resources to be excluded from config loading

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | #31516
| License       | MIT
| Doc PR        | not yet

The PR will fix the following RFC: #31516

Like resource loading for services, this PR offers a way to exclude patterns of resources like:

```yml
// config/routes/annotations.yaml
controllers:
    resource: ../../src/Controller/*
    type: annotation
    exclude: '../src/Controller/{DebugEmailController}.php'
```

All the annotation routes inside `Controller/` will be loaded in this example except all the one present inside the `Controller/DebugEmailController.php`

Commits
-------

332ff88 [Routing][Config] Allow patterns of resources to be excluded from config loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Feature Routing
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@nicolas-grekas @tristanbes @ro0NL @xabbuh @Kocal @vudaltsov