Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit fb532bf

Browse filesBrowse files
fabpotfabpot
committed
feature #22680 [DI] Fixing missing "exclude" functionality from PSR4 loader (weaverryan)
This PR was merged into the 3.3-dev branch. Discussion ---------- [DI] Fixing missing "exclude" functionality from PSR4 loader | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | none | License | MIT | Doc PR | TODO When the PSR4 loader was added in #21289, @nicolas-grekas said this: > given that glob() is powerful enough to include/exclude dirs, I removed the Test special exclusion (source: #21289 (comment)) But, I don't believe that's true! [Glob is all about inclusion, not exclusion](https://en.wikipedia.org/wiki/Glob_(programming)#Syntax) - the maximum you can exclude is a single character. Thus, I've marked this as a bug. My motivation came from upgrading KnpU to the new 3.3 DI stuff. We have many directories (40) in `src/`, and listing them all one-by-one in `resource:` is crazy - the `resource` line would be ~350 characters long and quite unreadable. What I really want to do is include *everything* and then exclude few directories. I tried to do this with `glob`, but it's not possible. This PR allows for something like this: ```yml services: _defaults: public: false # ... AppBundle\: resource: '../../src/AppBundle/*' exclude: '../../src/AppBundle/{AppBundle.php,Entity}' ``` This works *beautifully* in practice. And even if I forget to exclude a directory - since the services are private - **they're ultimately removed from the container anyways**. In fact, the *only* reason I need to exclude *anything* is because of the new "service argument resolver", which causes entities to not be properly removed from the compiled container. Thanks! Commits ------- 7d07f19 Allowing prototype/PSR4 service loading ability to exclude, because glob doesn't support this
2 parents 76190ab + 7d07f19 commit fb532bf
Copy full SHA for fb532bf

File tree

9 files changed

+100
-6
lines changed
Filter options

9 files changed

+100
-6
lines changed

‎src/Symfony/Component/DependencyInjection/Loader/FileLoader.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Loader/FileLoader.php
+27-3Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,9 @@ public function __construct(ContainerBuilder $container, FileLocatorInterface $l
4747
* @param Definition $prototype A definition to use as template
4848
* @param string $namespace The namespace prefix of classes in the scanned directory
4949
* @param string $resource The directory to look for classes, glob-patterns allowed
50+
* @param string $exclude A globed path of files to exclude
5051
*/
51-
public function registerClasses(Definition $prototype, $namespace, $resource)
52+
public function registerClasses(Definition $prototype, $namespace, $resource, $exclude = null)
5253
{
5354
if ('\\' !== substr($namespace, -1)) {
5455
throw new InvalidArgumentException(sprintf('Namespace prefix must end with a "\\": %s.', $namespace));
@@ -57,7 +58,7 @@ public function registerClasses(Definition $prototype, $namespace, $resource)
5758
throw new InvalidArgumentException(sprintf('Namespace is not a valid PSR-4 prefix: %s.', $namespace));
5859
}
5960

60-
$classes = $this->findClasses($namespace, $resource);
61+
$classes = $this->findClasses($namespace, $resource, $exclude);
6162
// prepare for deep cloning
6263
$prototype = serialize($prototype);
6364

@@ -84,16 +85,39 @@ protected function setDefinition($id, Definition $definition)
8485
}
8586
}
8687

87-
private function findClasses($namespace, $pattern)
88+
private function findClasses($namespace, $pattern, $excludePattern)
8889
{
8990
$parameterBag = $this->container->getParameterBag();
91+
92+
$excludePaths = array();
93+
$excludePrefix = null;
94+
if ($excludePattern) {
95+
$excludePattern = $parameterBag->unescapeValue($parameterBag->resolveValue($excludePattern));
96+
foreach ($this->glob($excludePattern, true, $resource) as $path => $info) {
97+
if (null === $excludePrefix) {
98+
$excludePrefix = $resource->getPrefix();
99+
}
100+
101+
// normalize Windows slashes
102+
$excludePaths[str_replace('\\', '/', $path)] = true;
103+
}
104+
}
105+
90106
$pattern = $parameterBag->unescapeValue($parameterBag->resolveValue($pattern));
91107
$classes = array();
92108
$extRegexp = defined('HHVM_VERSION') ? '/\\.(?:php|hh)$/' : '/\\.php$/';
93109
$prefixLen = null;
94110
foreach ($this->glob($pattern, true, $resource) as $path => $info) {
95111
if (null === $prefixLen) {
96112
$prefixLen = strlen($resource->getPrefix());
113+
114+
if ($excludePrefix && strpos($excludePrefix, $resource->getPrefix()) !== 0) {
115+
throw new InvalidArgumentException(sprintf('Invalid "exclude" pattern when importing classes for "%s": make sure your "exclude" pattern (%s) is a subset of the "resource" pattern (%s)', $namespace, $excludePattern, $pattern));
116+
}
117+
}
118+
119+
if (isset($excludePaths[str_replace('\\', '/', $path)])) {
120+
continue;
97121
}
98122

99123
if (!preg_match($extRegexp, $path, $m) || !$info->isReadable()) {

‎src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ private function parseDefinitions(\DOMDocument $xml, $file, $defaults)
143143
foreach ($services as $service) {
144144
if (null !== $definition = $this->parseDefinition($service, $file, $defaults)) {
145145
if ('prototype' === $service->tagName) {
146-
$this->registerClasses($definition, (string) $service->getAttribute('namespace'), (string) $service->getAttribute('resource'));
146+
$this->registerClasses($definition, (string) $service->getAttribute('namespace'), (string) $service->getAttribute('resource'), (string) $service->getAttribute('exclude'));
147147
} else {
148148
$this->setDefinition((string) $service->getAttribute('id'), $definition);
149149
}

‎src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php
+3-1Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ class YamlFileLoader extends FileLoader
6161

6262
private static $prototypeKeywords = array(
6363
'resource' => 'resource',
64+
'exclude' => 'exclude',
6465
'parent' => 'parent',
6566
'shared' => 'shared',
6667
'lazy' => 'lazy',
@@ -528,7 +529,8 @@ private function parseDefinition($id, $service, $file, array $defaults)
528529
if (!is_string($service['resource'])) {
529530
throw new InvalidArgumentException(sprintf('A "resource" attribute must be of type string for service "%s" in %s. Check your YAML syntax.', $id, $file));
530531
}
531-
$this->registerClasses($definition, $id, $service['resource']);
532+
$exclude = isset($service['exclude']) ? $service['exclude'] : null;
533+
$this->registerClasses($definition, $id, $service['resource'], $exclude);
532534
} else {
533535
$this->setDefinition($id, $definition);
534536
}

‎src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@
161161
</xsd:choice>
162162
<xsd:attribute name="namespace" type="xsd:string" use="required" />
163163
<xsd:attribute name="resource" type="xsd:string" use="required" />
164+
<xsd:attribute name="exclude" type="xsd:string" />
164165
<xsd:attribute name="shared" type="boolean" />
165166
<xsd:attribute name="public" type="boolean" />
166167
<xsd:attribute name="lazy" type="boolean" />

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/Prototype/OtherDir/AnotherSub/DeeperBaz.php

Copy file name to clipboard
+7Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<?php
2+
3+
namespace Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\OtherDir\AnotherSub;
4+
5+
class DeeperBaz
6+
{
7+
}

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/Prototype/OtherDir/Baz.php

Copy file name to clipboard
+7Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<?php
2+
3+
namespace Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\OtherDir;
4+
5+
class Baz
6+
{
7+
}

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services_prototype.xml

Copy file name to clipboard
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<?xml version="1.0" encoding="utf-8"?>
22
<container xmlns="http://symfony.com/schema/dic/services" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
33
<services>
4-
<prototype namespace="Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\" resource="../Prototype/*" />
4+
<prototype namespace="Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\" resource="../Prototype/*" exclude="../Prototype/{OtherDir}" />
55
</services>
66
</container>

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_prototype.yml

Copy file name to clipboard
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
services:
22
Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\:
33
resource: ../Prototype
4+
exclude: '../Prototype/{OtherDir}'

‎src/Symfony/Component/DependencyInjection/Tests/Loader/FileLoaderTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/Loader/FileLoaderTest.php
+52Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,15 @@
1616
use Symfony\Component\Config\Loader\LoaderResolver;
1717
use Symfony\Component\DependencyInjection\ContainerBuilder;
1818
use Symfony\Component\DependencyInjection\Definition;
19+
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
1920
use Symfony\Component\DependencyInjection\Loader\FileLoader;
2021
use Symfony\Component\DependencyInjection\Loader\IniFileLoader;
2122
use Symfony\Component\DependencyInjection\Loader\PhpFileLoader;
2223
use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
2324
use Symfony\Component\DependencyInjection\Loader\YamlFileLoader;
2425
use Symfony\Component\DependencyInjection\Reference;
26+
use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\OtherDir\AnotherSub\DeeperBaz;
27+
use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\OtherDir\Baz;
2528
use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\Bar;
2629

2730
class FileLoaderTest extends TestCase
@@ -90,6 +93,26 @@ public function testRegisterClasses()
9093
);
9194
}
9295

96+
public function testRegisterClassesWithExclude()
97+
{
98+
$container = new ContainerBuilder();
99+
$container->setParameter('other_dir', 'OtherDir');
100+
$loader = new TestFileLoader($container, new FileLocator(self::$fixturesPath.'/Fixtures'));
101+
102+
$loader->registerClasses(
103+
new Definition(),
104+
'Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\\',
105+
'Prototype/*',
106+
// load everything, except OtherDir/AnotherSub & Foo.php
107+
'Prototype/{%other_dir%/AnotherSub,Foo.php}'
108+
);
109+
110+
$this->assertTrue($container->has(Bar::class));
111+
$this->assertTrue($container->has(Baz::class));
112+
$this->assertFalse($container->has(Foo::class));
113+
$this->assertFalse($container->has(DeeperBaz::class));
114+
}
115+
93116
/**
94117
* @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException
95118
* @expectedExceptionMessageRegExp /Expected to find class "Symfony\\Component\\DependencyInjection\\Tests\\Fixtures\\Prototype\\Bar" in file ".+" while importing services from resource "Prototype\/Sub\/\*", but it was not found\! Check the namespace prefix used with the resource/
@@ -102,6 +125,35 @@ public function testRegisterClassesWithBadPrefix()
102125
// the Sub is missing from namespace prefix
103126
$loader->registerClasses(new Definition(), 'Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\\', 'Prototype/Sub/*');
104127
}
128+
129+
/**
130+
* @dataProvider getIncompatibleExcludeTests
131+
*/
132+
public function testRegisterClassesWithIncompatibleExclude($resourcePattern, $excludePattern)
133+
{
134+
$container = new ContainerBuilder();
135+
$loader = new TestFileLoader($container, new FileLocator(self::$fixturesPath.'/Fixtures'));
136+
137+
try {
138+
$loader->registerClasses(
139+
new Definition(),
140+
'Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\\',
141+
$resourcePattern,
142+
$excludePattern
143+
);
144+
} catch (InvalidArgumentException $e) {
145+
$this->assertEquals(
146+
sprintf('Invalid "exclude" pattern when importing classes for "Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\": make sure your "exclude" pattern (%s) is a subset of the "resource" pattern (%s)', $excludePattern, $resourcePattern),
147+
$e->getMessage()
148+
);
149+
}
150+
}
151+
152+
public function getIncompatibleExcludeTests()
153+
{
154+
yield array('Prototype/*', 'yaml/*', false);
155+
yield array('Prototype/OtherDir/*', 'Prototype/*', false);
156+
}
105157
}
106158

107159
class TestFileLoader extends FileLoader

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.