Skip to content

Commit

Permalink
[Config] [DependencyInjection] [Routing] Added option `ignore_not_fou…
Browse files Browse the repository at this point in the history
…nd` for imported config files
  • Loading branch information
pulzarraider authored and drejk committed May 4, 2019
1 parent 8fdcd6e commit c7916cf
Show file tree
Hide file tree
Showing 24 changed files with 158 additions and 18 deletions.
7 changes: 7 additions & 0 deletions UPGRADE-4.4.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
UPGRADE FROM 4.3 to 4.4
=======================

Config
------

* Passing a boolean to argument `$ignoreErrors` of `FileLoader::import()` is deprecated, pass an `int` instead
1 change: 1 addition & 0 deletions UPGRADE-5.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ Config
* Removed `FileLoaderLoadException`, use `LoaderLoadException` instead.
* Using environment variables with `cannotBeEmpty()` if the value is validated with `validate()` will throw an exception.
* Removed the `root()` method in `TreeBuilder`, pass the root node information to the constructor instead
* Argument `$ignoreErrors` of `FileLoader::import()` now accepts `int` instead of `bool`

Console
-------
Expand Down
6 changes: 6 additions & 0 deletions src/Symfony/Component/Config/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
CHANGELOG
=========

4.4.0
-----

* added option `ignore_not_found` for imported config files
* deprecated passing a boolean to argument `$ignoreErrors` of `FileLoader::import()`, pass an `int` instead

4.3.0
-----

Expand Down
26 changes: 20 additions & 6 deletions src/Symfony/Component/Config/Loader/FileLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@
*/
abstract class FileLoader extends Loader
{
const IGNORE_NONE = 0;
const IGNORE_ALL = 1;
const IGNORE_FILE_NOT_FOUND = 2;

protected static $loading = [];

protected $locator;
Expand Down Expand Up @@ -61,7 +65,7 @@ public function getLocator()
*
* @param mixed $resource A Resource
* @param string|null $type The resource type or null if unknown
* @param bool $ignoreErrors Whether to ignore import errors or not
* @param int $ignoreErrors Whether to ignore import errors or not
* @param string|null $sourceResource The original resource importing the new resource
*
* @return mixed
Expand All @@ -70,8 +74,14 @@ public function getLocator()
* @throws FileLoaderImportCircularReferenceException
* @throws FileLocatorFileNotFoundException
*/
public function import($resource, $type = null, $ignoreErrors = false, $sourceResource = null)
public function import($resource, $type = null, $ignoreErrors = self::IGNORE_NONE, $sourceResource = null)
{
if (!\is_int($ignoreErrors)) {
@trigger_error(sprintf('Passing a boolean to argument "$ignoreErrors" of "%s()" is deprecated since Symfony 4.4, pass an integer instead.', __METHOD__), E_USER_DEPRECATED);

$ignoreErrors = $ignoreErrors ? parent::IGNORE_ALL : parent::IGNORE_NONE;
}

if (\is_string($resource) && \strlen($resource) !== $i = strcspn($resource, '*?{[')) {
$ret = [];
$isSubpath = 0 !== $i && false !== strpos(substr($resource, 0, $i), '/');
Expand All @@ -93,7 +103,7 @@ public function import($resource, $type = null, $ignoreErrors = false, $sourceRe
/**
* @internal
*/
protected function glob(string $pattern, bool $recursive, &$resource = null, bool $ignoreErrors = false, bool $forExclusion = false, array $excluded = [])
protected function glob(string $pattern, bool $recursive, &$resource = null, int $ignoreErrors = self::IGNORE_NONE, bool $forExclusion = false, array $excluded = [])
{
if (\strlen($pattern) === $i = strcspn($pattern, '*?{[')) {
$prefix = $pattern;
Expand All @@ -109,7 +119,7 @@ protected function glob(string $pattern, bool $recursive, &$resource = null, boo
try {
$prefix = $this->locator->locate($prefix, $this->currentDir, true);
} catch (FileLocatorFileNotFoundException $e) {
if (!$ignoreErrors) {
if (self::IGNORE_ALL !== $ignoreErrors) {
throw $e;
}

Expand All @@ -125,7 +135,7 @@ protected function glob(string $pattern, bool $recursive, &$resource = null, boo
yield from $resource;
}

private function doImport($resource, $type = null, bool $ignoreErrors = false, $sourceResource = null)
private function doImport($resource, $type = null, int $ignoreErrors, $sourceResource)
{
try {
$loader = $this->resolve($resource, $type);
Expand Down Expand Up @@ -157,7 +167,11 @@ private function doImport($resource, $type = null, bool $ignoreErrors = false, $
} catch (FileLoaderImportCircularReferenceException $e) {
throw $e;
} catch (\Exception $e) {
if (!$ignoreErrors) {
if (self::IGNORE_ALL !== $ignoreErrors) {
if ($e instanceof FileLocatorFileNotFoundException && self::IGNORE_FILE_NOT_FOUND === $ignoreErrors) {
return;
}

// prevent embedded imports from nesting multiple exceptions
if ($e instanceof LoaderLoadException) {
throw $e;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ final public function extension(string $namespace, array $config)
$this->container->loadFromExtension($namespace, static::processValue($config));
}

final public function import(string $resource, string $type = null, bool $ignoreErrors = false)
final public function import(string $resource, string $type = null, int $ignoreErrors = PhpFileLoader::IGNORE_NONE)
{
$this->loader->setCurrentDir(\dirname($this->path));
$this->loader->import($resource, $type, $ignoreErrors, $this->file);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public function load($file, $type = null)

$this->setCurrentDir($path);

$this->import($dir, null, false, $path);
$this->import($dir, null, parent::IGNORE_NONE, $path);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,16 @@ private function parseImports(\DOMDocument $xml, $file)
$defaultDirectory = \dirname($file);
foreach ($imports as $import) {
$this->setCurrentDir($defaultDirectory);
$this->import($import->getAttribute('resource'), XmlUtils::phpize($import->getAttribute('type')) ?: null, (bool) XmlUtils::phpize($import->getAttribute('ignore-errors')), $file);

if (XmlUtils::phpize($import->getAttribute('ignore-errors'))) {
$ignoreErrors = parent::IGNORE_ALL;
} elseif (XmlUtils::phpize($import->getAttribute('ignore-not-found'))) {
$ignoreErrors = parent::IGNORE_FILE_NOT_FOUND;
} else {
$ignoreErrors = parent::IGNORE_NONE;
}

$this->import($import->getAttribute('resource'), XmlUtils::phpize($import->getAttribute('type')) ?: null, $ignoreErrors, $file);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,16 @@ private function parseImports(array $content, string $file)
}

$this->setCurrentDir($defaultDirectory);
$this->import($import['resource'], isset($import['type']) ? $import['type'] : null, isset($import['ignore_errors']) ? (bool) $import['ignore_errors'] : false, $file);

if (!empty($import['ignore_errors'])) {
$ignoreErrors = parent::IGNORE_ALL;
} elseif (!empty($import['ignore_not_found'])) {
$ignoreErrors = parent::IGNORE_FILE_NOT_FOUND;
} else {
$ignoreErrors = parent::IGNORE_NONE;
}

$this->import($import['resource'], $import['type'] ?? null, $ignoreErrors, $file);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@
</xsd:annotation>
<xsd:attribute name="resource" type="xsd:string" use="required" />
<xsd:attribute name="ignore-errors" type="boolean" />
<xsd:attribute name="ignore-not-found" type="boolean" />
<xsd:attribute name="type" type="xsd:string" />
</xsd:complexType>

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?xml version="1.0" ?>

<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 https://symfony.com/schema/dic/services/services-1.0.xsd">
<imports>
<import resource="foo_fake.xml" ignore-not-found="true" />
</imports>
</container>
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?xml version="1.0" ?>

<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 https://symfony.com/schema/dic/services/services-1.0.xsd">
<imports>
<import resource="nonvalid.xml" ignore-not-found="true" />
</imports>
</container>
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?xml version="1.0" ?>

<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 https://symfony.com/schema/dic/services/services-1.0.xsd">
<imports>
<import resource="foo_fake.xml" />
</imports>
</container>
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
imports:
- { resource: foo_fake.yml, ignore_not_found: true }
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
imports:
- { resource: nonvalid2.yml, ignore_not_found: true }
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
imports:
- { resource: foo_fake.yml }
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use PHPUnit\Framework\TestCase;
use Symfony\Component\Config\FileLocator;
use Symfony\Component\Config\Loader\FileLoader;
use Symfony\Component\Config\Resource\GlobResource;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Loader\GlobFileLoader;
Expand All @@ -38,7 +39,7 @@ public function testLoadAddsTheGlobResourceToTheContainer()

class GlobFileLoaderWithoutImport extends GlobFileLoader
{
public function import($resource, $type = null, $ignoreErrors = false, $sourceResource = null)
public function import($resource, $type = null, $ignoreErrors = FileLoader::IGNORE_NONE, $sourceResource = null)
{
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,37 @@ public function testLoadImports()

// Bad import throws no exception due to ignore_errors value.
$loader->load('services4_bad_import.xml');

// Bad import with nonexistent file throws no exception due to ignore_not_found value.
$loader->load('services4_bad_import_file_not_found.xml');

try {
$loader->load('services4_bad_import_with_errors.xml');
$this->fail('->load() throws a LoaderLoadException if the imported xml file configuration does not exist');
} catch (\Exception $e) {
$this->assertInstanceOf('Symfony\\Component\\Config\\Exception\\LoaderLoadException', $e, '->load() throws a LoaderLoadException if the imported xml file configuration does not exist');
$this->assertRegExp(sprintf('#^The file "%1$s" does not exist \(in: .+\) in %1$s \(which is being imported from ".+%2$s"\)\.$#', 'foo_fake\.xml', 'services4_bad_import_with_errors\.xml'), $e->getMessage(), '->load() throws a LoaderLoadException if the imported xml file configuration does not exist');

$e = $e->getPrevious();
$this->assertInstanceOf('Symfony\\Component\\Config\\Exception\\FileLocatorFileNotFoundException', $e, '->load() throws a FileLocatorFileNotFoundException if the imported xml file configuration does not exist');
$this->assertRegExp(sprintf('#^The file "%s" does not exist \(in: .+\)\.$#', 'foo_fake\.xml'), $e->getMessage(), '->load() throws a FileLocatorFileNotFoundException if the imported xml file configuration does not exist');
}

try {
$loader->load('services4_bad_import_nonvalid.xml');
$this->fail('->load() throws an LoaderLoadException if the imported configuration does not validate the XSD');
} catch (\Exception $e) {
$this->assertInstanceOf('Symfony\\Component\\Config\\Exception\\LoaderLoadException', $e, '->load() throws a LoaderLoadException if the imported configuration does not validate the XSD');
$this->assertRegExp(sprintf('#^Unable to parse file ".+%s": .+.$#', 'nonvalid\.xml'), $e->getMessage(), '->load() throws a LoaderLoadException if the imported configuration does not validate the XSD');

$e = $e->getPrevious();
$this->assertInstanceOf('Symfony\\Component\\DependencyInjection\\Exception\\InvalidArgumentException', $e, '->load() throws an InvalidArgumentException if the configuration does not validate the XSD');
$this->assertRegExp(sprintf('#^Unable to parse file ".+%s": .+.$#', 'nonvalid\.xml'), $e->getMessage(), '->load() throws an InvalidArgumentException if the configuration does not validate the XSD');

$e = $e->getPrevious();
$this->assertInstanceOf('Symfony\\Component\\Config\\Util\\Exception\\XmlParsingException', $e, '->load() throws a XmlParsingException if the configuration does not validate the XSD');
$this->assertStringStartsWith('[ERROR 1845] Element \'nonvalid\': No matching global declaration available for the validation root. (in', $e->getMessage(), '->load() throws a XmlParsingException if the loaded file does not validate the XSD');
}
}

public function testLoadAnonymousServices()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,33 @@ public function testLoadImports()

// Bad import throws no exception due to ignore_errors value.
$loader->load('services4_bad_import.yml');

// Bad import with nonexistent file throws no exception due to ignore_not_found value.
$loader->load('services4_bad_import_file_not_found.yml');

try {
$loader->load('services4_bad_import_with_errors.yml');
$this->fail('->load() throws a LoaderLoadException if the imported yaml file does not exist');
} catch (\Exception $e) {
$this->assertInstanceOf('Symfony\\Component\\Config\\Exception\\LoaderLoadException', $e, '->load() throws a LoaderLoadException if the imported yaml file does not exist');
$this->assertRegExp(sprintf('#^The file "%1$s" does not exist \(in: .+\) in %1$s \(which is being imported from ".+%2$s"\)\.$#', 'foo_fake\.yml', 'services4_bad_import_with_errors\.yml'), $e->getMessage(), '->load() throws a LoaderLoadException if the imported yaml file does not exist');

$e = $e->getPrevious();
$this->assertInstanceOf('Symfony\\Component\\Config\\Exception\\FileLocatorFileNotFoundException', $e, '->load() throws a FileLocatorFileNotFoundException if the imported yaml file does not exist');
$this->assertRegExp(sprintf('#^The file "%s" does not exist \(in: .+\)\.$#', 'foo_fake\.yml'), $e->getMessage(), '->load() throws a FileLocatorFileNotFoundException if the imported yaml file does not exist');
}

try {
$loader->load('services4_bad_import_nonvalid.yml');
$this->fail('->load() throws a LoaderLoadException if the tag in the imported yaml file is not valid');
} catch (\Exception $e) {
$this->assertInstanceOf('Symfony\\Component\\Config\\Exception\\LoaderLoadException', $e, '->load() throws a LoaderLoadException if the tag in the imported yaml file is not valid');
$this->assertRegExp(sprintf('#^The service file ".+%1$s" is not valid\. It should contain an array\. Check your YAML syntax in .+%1$s \(which is being imported from ".+%2$s"\)\.$#', 'nonvalid2\.yml', 'services4_bad_import_nonvalid.yml'), $e->getMessage(), '->load() throws a LoaderLoadException if the tag in the imported yaml file is not valid');

$e = $e->getPrevious();
$this->assertInstanceOf('Symfony\\Component\\DependencyInjection\\Exception\\InvalidArgumentException', $e, '->load() throws an InvalidArgumentException if the tag in the imported yaml file is not valid');
$this->assertRegExp(sprintf('#^The service file ".+%s" is not valid\. It should contain an array\. Check your YAML syntax\.$#', 'nonvalid2\.yml'), $e->getMessage(), '->load() throws an InvalidArgumentException if the tag in the imported yaml file is not valid');
}
}

public function testLoadServices()
Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Component/DependencyInjection/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
},
"require-dev": {
"symfony/yaml": "~3.4|~4.0",
"symfony/config": "^4.3",
"symfony/config": "^4.4",
"symfony/expression-language": "~3.4|~4.0"
},
"suggest": {
Expand All @@ -33,7 +33,7 @@
"symfony/proxy-manager-bridge": "Generate service proxies to lazy load them"
},
"conflict": {
"symfony/config": "<4.3",
"symfony/config": "<4.4",
"symfony/finder": "<3.4",
"symfony/proxy-manager-bridge": "<3.4",
"symfony/yaml": "<3.4"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public function __construct(RouteCollection $collection, PhpFileLoader $loader,
/**
* @return ImportConfigurator
*/
final public function import($resource, $type = null, $ignoreErrors = false)
final public function import($resource, $type = null, int $ignoreErrors = PhpFileLoader::IGNORE_NONE)
{
$this->loader->setCurrentDir(\dirname($this->path));
$imported = $this->loader->import($resource, $type, $ignoreErrors, $this->file);
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Routing/Loader/DirectoryLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public function load($file, $type = null)
$subType = 'directory';
}

$subCollection = $this->import($subPath, $subType, false, $path);
$subCollection = $this->import($subPath, $subType, parent::IGNORE_NONE, $path);
$collection->addCollection($subCollection);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Routing/Loader/XmlFileLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ protected function parseImport(RouteCollection $collection, \DOMElement $node, $
$this->setCurrentDir(\dirname($path));

/** @var RouteCollection[] $imported */
$imported = $this->import($resource, ('' !== $type ? $type : null), false, $file);
$imported = $this->import($resource, ('' !== $type ? $type : null), parent::IGNORE_NONE, $file);

if (!\is_array($imported)) {
$imported = [$imported];
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Routing/Loader/YamlFileLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ protected function parseImport(RouteCollection $collection, array $config, $path

$this->setCurrentDir(\dirname($path));

$imported = $this->import($config['resource'], $type, false, $file);
$imported = $this->import($config['resource'], $type, parent::IGNORE_NONE, $file);

if (!\is_array($imported)) {
$imported = [$imported];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use PHPUnit\Framework\TestCase;
use Symfony\Component\Config\FileLocator;
use Symfony\Component\Config\Loader\FileLoader;
use Symfony\Component\Config\Resource\GlobResource;
use Symfony\Component\Routing\Loader\GlobFileLoader;
use Symfony\Component\Routing\RouteCollection;
Expand All @@ -38,7 +39,7 @@ public function testLoadAddsTheGlobResourceToTheContainer()

class GlobFileLoaderWithoutImport extends GlobFileLoader
{
public function import($resource, $type = null, $ignoreErrors = false, $sourceResource = null)
public function import($resource, $type = null, $ignoreErrors = FileLoader::IGNORE_NONE, $sourceResource = null)
{
return new RouteCollection();
}
Expand Down

0 comments on commit c7916cf

Please sign in to comment.