Skip to content

[DI] More generic loaders #21062

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

Closed
wants to merge 1 commit into from
Closed

[DI] More generic loaders #21062

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 30 additions & 13 deletions src/Symfony/Component/Config/Loader/FileLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ abstract class FileLoader extends Loader
*/
protected $locator;

private $currentResource;
private $currentDir;

/**
Expand All @@ -49,11 +50,22 @@ public function __construct(FileLocatorInterface $locator)
*
* @param string $dir
*/
public function setCurrentDir($dir)
public function setCurrentDir($dir) //deprecate
{
$this->currentDir = $dir;
}

/**
* Sets the current resource being loaded.
*
* @param string $resource
*/
public function setCurrentResource($resource)
{
$this->currentResource = $this->locate($resource);
$this->currentDir = '/' === substr($this->currentResource, -1) || is_dir($this->currentResource) ? rtrim($this->currentResource, '/') : dirname($this->currentResource);
}

/**
* Returns the file locator used by this loader.
*
Expand All @@ -65,25 +77,23 @@ public function getLocator()
}

/**
* Imports a resource.
*
* @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 string|null $sourceResource The original resource importing the new resource
*
* @return mixed
* {@inheritdoc}
*
* @throws FileLoaderLoadException
* @throws FileLoaderImportCircularReferenceException
*/
public function import($resource, $type = null, $ignoreErrors = false, $sourceResource = null)
public function import($resource, $type = null, $ignoreErrors = false, $sourceResource = null) // deprecate $sourceResource
{
$currentResource = $sourceResource ?: $this->currentResource;
$currentDir = $this->currentDir;

try {
$loader = $this->resolve($resource, $type);

if ($loader instanceof self && null !== $this->currentDir) {
$resource = $loader->getLocator()->locate($resource, $this->currentDir, false);
if ($loader instanceof self && $this !== $loader) {
$resource = $loader->getLocator()->locate($resource, $currentDir, false);
} else {
$resource = $this->locate($resource, $currentDir, false);
}

$resources = is_array($resource) ? $resource : array($resource);
Expand All @@ -100,9 +110,11 @@ public function import($resource, $type = null, $ignoreErrors = false, $sourceRe
self::$loading[$resource] = true;

try {
$this->setCurrentResource($resource);
$ret = $loader->load($resource, $type);
} finally {
unset(self::$loading[$resource]);
$this->setCurrentResource($currentResource);
Copy link
Member

Choose a reason for hiding this comment

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

this is wrong, as it does not reset the object in the previous state (as $currentResource is not set directly in the property here, and may not come from the property either).

Adding 1 more state property in the loader is a bad idea IMO, as state in service objects makes them harder to use.

}

return $ret;
Expand All @@ -115,8 +127,13 @@ public function import($resource, $type = null, $ignoreErrors = false, $sourceRe
throw $e;
}

throw new FileLoaderLoadException($resource, $sourceResource, null, $e);
throw new FileLoaderLoadException($resource, $currentResource, null, $e);
}
}
}

protected function locate($file, $currentDir = null, $first = true)
{
return $this->locator->locate($file, $currentDir ?: $this->currentDir, $first);
}
}
29 changes: 29 additions & 0 deletions src/Symfony/Component/Config/Loader/ImportingLoaderInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Config\Loader;

/**
* @author Roland Franssen <franssen.roland@gmail.com>
*/
interface ImportingLoaderInterface extends LoaderInterface
{
/**
* Imports a resource.
*
* @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
*
* @return mixed
*/
public function import($resource, $type = null, $ignoreErrors = false);
}
21 changes: 12 additions & 9 deletions src/Symfony/Component/Config/Loader/Loader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
*
* @author Fabien Potencier <fabien@symfony.com>
*/
abstract class Loader implements LoaderInterface
abstract class Loader implements ImportingLoaderInterface
{
protected $resolver;

Expand All @@ -39,16 +39,19 @@ public function setResolver(LoaderResolverInterface $resolver)
}

/**
* Imports a resource.
*
* @param mixed $resource A resource
* @param string|null $type The resource type or null if unknown
* {@inheritdoc}
*
* @return mixed
* @throws \Exception
*/
public function import($resource, $type = null)
public function import($resource, $type = null, $ignoreErrors = false)
{
return $this->resolve($resource, $type)->load($resource, $type);
try {
return $this->resolve($resource, $type)->load($resource, $type);
} catch (\Exception $e) {
if (!$ignoreErrors) {
throw $e;
}
}
}

/**
Expand All @@ -70,7 +73,7 @@ public function resolve($resource, $type = null)
$loader = null === $this->resolver ? false : $this->resolver->resolve($resource, $type);

if (false === $loader) {
throw new FileLoaderLoadException($resource);
throw new FileLoaderLoadException($resource); // @FIXME
}

return $loader;
Expand Down
11 changes: 9 additions & 2 deletions src/Symfony/Component/Config/Tests/Loader/FileLoaderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Component\Config\Tests\Loader;

use Symfony\Component\Config\FileLocatorInterface;
use Symfony\Component\Config\Loader\FileLoader;
use Symfony\Component\Config\Loader\LoaderResolver;

Expand All @@ -22,6 +23,7 @@ public function testImportWithFileLocatorDelegation()

$locatorMockForAdditionalLoader = $this->getMockBuilder('Symfony\Component\Config\FileLocatorInterface')->getMock();
$locatorMockForAdditionalLoader->expects($this->any())->method('locate')->will($this->onConsecutiveCalls(
'path/to/file1', // Default
array('path/to/file1'), // Default
array('path/to/file1', 'path/to/file2'), // First is imported
array('path/to/file1', 'path/to/file2'), // Second is imported
Expand All @@ -31,10 +33,8 @@ public function testImportWithFileLocatorDelegation()

$fileLoader = new TestFileLoader($locatorMock);
$fileLoader->setSupports(false);
$fileLoader->setCurrentDir('.');

$additionalLoader = new TestFileLoader($locatorMockForAdditionalLoader);
$additionalLoader->setCurrentDir('.');

$fileLoader->setResolver($loaderResolver = new LoaderResolver(array($fileLoader, $additionalLoader)));

Expand Down Expand Up @@ -71,6 +71,13 @@ class TestFileLoader extends FileLoader
{
private $supports = true;

public function __construct(FileLocatorInterface $locator, $currentResource = './foo')
{
parent::__construct($locator);

$this->setCurrentResource($currentResource);
}

public function load($resource, $type = null)
{
return $resource;
Expand Down
11 changes: 3 additions & 8 deletions src/Symfony/Component/DependencyInjection/Loader/DirectoryLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@

namespace Symfony\Component\DependencyInjection\Loader;

use Symfony\Component\Config\Resource\DirectoryResource;

/**
* DirectoryLoader is a recursive loader to go through directories.
*
Expand All @@ -25,19 +23,16 @@ class DirectoryLoader extends FileLoader
*/
public function load($file, $type = null)
{
$file = rtrim($file, '/');
$path = $this->locator->locate($file);
$this->container->addResource(new DirectoryResource($path));
$this->setCurrentResource($file);

$path = $this->locate(rtrim($file, '/'));
foreach (scandir($path) as $dir) {
if ('.' !== $dir[0]) {
if (is_dir($path.'/'.$dir)) {
$dir .= '/'; // append / to allow recursion
}

$this->setCurrentDir($path);

$this->import($dir, null, false, $path);
$this->import($dir);
}
}
}
Expand Down
11 changes: 11 additions & 0 deletions src/Symfony/Component/DependencyInjection/Loader/FileLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

namespace Symfony\Component\DependencyInjection\Loader;

use Symfony\Component\Config\Resource\DirectoryResource;
use Symfony\Component\Config\Resource\FileResource;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\Config\Loader\FileLoader as BaseFileLoader;
use Symfony\Component\Config\FileLocatorInterface;
Expand All @@ -34,4 +36,13 @@ public function __construct(ContainerBuilder $container, FileLocatorInterface $l

parent::__construct($locator);
}

public function setCurrentResource($resource)
{
parent::setCurrentResource($resource);

$path = $this->locate($resource);

$this->container->addResource('/' === substr($path, -1) || is_dir($path) ? new DirectoryResource($path) : new FileResource($path));
}
}
5 changes: 2 additions & 3 deletions src/Symfony/Component/DependencyInjection/Loader/IniFileLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

namespace Symfony\Component\DependencyInjection\Loader;

use Symfony\Component\Config\Resource\FileResource;
use Symfony\Component\Config\Util\XmlUtils;
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;

Expand All @@ -27,9 +26,9 @@ class IniFileLoader extends FileLoader
*/
public function load($resource, $type = null)
{
$path = $this->locator->locate($resource);
$this->setCurrentResource($resource);

$this->container->addResource(new FileResource($path));
$path = $this->locate($resource);

// first pass to catch parsing errors
$result = parse_ini_file($path, true);
Expand Down
10 changes: 3 additions & 7 deletions src/Symfony/Component/DependencyInjection/Loader/PhpFileLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@

namespace Symfony\Component\DependencyInjection\Loader;

use Symfony\Component\Config\Resource\FileResource;

/**
* PhpFileLoader loads service definitions from a PHP file.
*
Expand All @@ -28,15 +26,13 @@ class PhpFileLoader extends FileLoader
*/
public function load($resource, $type = null)
{
$this->setCurrentResource($resource);

// the container and loader variables are exposed to the included file below
$container = $this->container;
$loader = $this;

$path = $this->locator->locate($resource);
$this->setCurrentDir(dirname($path));
$this->container->addResource(new FileResource($path));

include $path;
include $this->locate($resource);
}

/**
Expand Down
11 changes: 4 additions & 7 deletions src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

namespace Symfony\Component\DependencyInjection\Loader;

use Symfony\Component\Config\Resource\FileResource;
use Symfony\Component\Config\Util\XmlUtils;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\DependencyInjection\Alias;
Expand All @@ -36,11 +35,11 @@ class XmlFileLoader extends FileLoader
*/
public function load($resource, $type = null)
{
$path = $this->locator->locate($resource);
$this->setCurrentResource($resource);

$xml = $this->parseFileToDOM($path);
$path = $this->locate($resource);

$this->container->addResource(new FileResource($path));
$xml = $this->parseFileToDOM($path);

// anonymous services
$this->processAnonymousServices($xml, $path);
Expand Down Expand Up @@ -93,10 +92,8 @@ private function parseImports(\DOMDocument $xml, $file)
return;
}

$defaultDirectory = dirname($file);
foreach ($imports as $import) {
$this->setCurrentDir($defaultDirectory);
$this->import($import->getAttribute('resource'), null, (bool) XmlUtils::phpize($import->getAttribute('ignore-errors')), $file);
$this->import($import->getAttribute('resource'), null, (bool) XmlUtils::phpize($import->getAttribute('ignore-errors')));
}
}

Expand Down
13 changes: 5 additions & 8 deletions src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
use Symfony\Component\Config\Resource\FileResource;
use Symfony\Component\Yaml\Exception\ParseException;
use Symfony\Component\Yaml\Parser as YamlParser;
use Symfony\Component\Yaml\Yaml;
Expand Down Expand Up @@ -64,14 +63,14 @@ class YamlFileLoader extends FileLoader
*/
public function load($resource, $type = null)
{
$path = $this->locator->locate($resource);
$this->setCurrentResource($resource);

$content = $this->loadFile($path);
$path = $this->locate($resource);

$this->container->addResource(new FileResource($path));
$content = $this->loadFile($path);

// empty file
if (null === $content) {
if (null === $content || !is_array($content)) {
return;
}

Expand Down Expand Up @@ -120,14 +119,12 @@ private function parseImports($content, $file)
throw new InvalidArgumentException(sprintf('The "imports" key should contain an array in %s. Check your YAML syntax.', $file));
}

$defaultDirectory = dirname($file);
foreach ($content['imports'] as $import) {
if (!is_array($import)) {
throw new InvalidArgumentException(sprintf('The values in the "imports" key should be arrays in %s. Check your YAML syntax.', $file));
}

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

Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/DependencyInjection/Tests/Loader/DirectoryLoaderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public function testImports()

/**
* @expectedException \InvalidArgumentException
* @expectedExceptionMessage The file "foo" does not exist (in:
* @expectedExceptionMessage The file "foo/" does not exist (in:
*/
public function testExceptionIsRaisedWhenDirectoryDoesNotExist()
{
Expand Down
Loading