8000 Use glob pattern to include routing annotations by ebuildy · Pull Request #25633 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

Use glob pattern to include routing annotations #25633

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.

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

Closed
wants to merge 2 commits into from
Closed

Use glob pattern to include routing annotations #25633

wants to merge 2 commits into from

Conversation

ebuildy
Copy link
Contributor
@ebuildy ebuildy commented Dec 30, 2017

FileLoader::import method can return an array, this PR fix the error "Call to a member function addPrefix() on array".

Use case "I want to import routes from controllers in different directories"; given a route/annotation.yaml like this:

controllers:
    resource: ../../src/*/Controller/*Controller.php
    type: annotation

(/*Controller.php is required to make the glob return something).

I dont know how to code this properly, that why I just copied/paste the code like this, but the idea is here.

Q A
Branch? master for features / 2.7 up to 4.0 for bug fixes
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes/no
Fixed tickets #...
License MIT
Doc PR symfony/symfony-docs#...

``FileLoader::import`` method can return an array, this PR fix the error "Call to a member function addPrefix() on array".

Use case "I want to import routes from controllers in different directories"; given a ``route/annotation.yaml`` like this:

```
controllers:
    resource: ../../src/*/Controller/*Controller.php
    type: annotation
```

(``/*Controller.php`` is required to make the glob return something).

I dont know how to code this properly, that why I just copied/paste the code like this, but the idea is here.

if (is_array($subCollection)) {
/* @var $value RouteCollection */
foreach($subCollection as $value) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This creates a lot of duplicate code. What if you simply ensure it's always an array?

if (!is_array($subCollection)) {
    $subCollection = array($subCollection);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

absolutely, I didnt know the best way to deal with, thanks you.

@nicolas-grekas
Copy link
Member

Could you add a test case please? If this is a bug fix, it should target the lowest maintained branch where the bug exists (either 2.7, 2.8, 3.3 or 3.4). Any hint in that aspect?

@ebuildy
Copy link
Contributor Author
ebuildy commented Dec 30, 2017

Sure, I have taken a look on Tests folder but dont know really how to implement this, I got error:

Cannot load resource "/src/Symfony/Component/Routing/Tests/Fixtures/ControllerClasses/Sub1/Sub1Controller.php". Make sure annotations are installed and enabled.

@nicolas-grekas
8000 Copy link
Member

fix the error "Call to a member function addPrefix() on array".

so, this is a bug fix, isn't it?

Make sure annotations are installed and enabled

please push a test case that you'd like to make work and we will be able to help I guess

@nicolas-grekas
Copy link
Member

Oh, please also ensure that other loaders don't have the same issue.

@nicolas-grekas
Copy link
Member

Closing in favor of #26600, thank you for working on this anyway!

nicolas-grekas added a commit that referenced this pull request Mar 22, 2018
… that match multiple resources (skalpa)

This PR was squashed before being merged into the 3.4 branch (closes #26600).

Discussion
----------

[Routing] Fixed the importing of files using glob patterns that match multiple resources

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #22160
| License       | MIT
| Doc PR        | n/a

This fixes the import of resources specified using glob patterns in `XmlFileLoader` and `YamlFileLoader`.

@nicolas-grekas This supersedes #25633 that's been in limbo since December despite your comments, so I decided to take care of it as I need this to work. I took care of the two loaders that are affected, and added tests.

Commits
-------

948b4cf [Routing] Fixed the importing of files using glob patterns that match multiple resources
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0