8000 [DI] Add ContainerBuilder::fileExists() for checking/tracking resource existence by chalasr · Pull Request #21408 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[DI] Add ContainerBuilder::fileExists() for checking/tracking resource existence #21408

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

Merged
merged 1 commit into from
Feb 2, 2017
Merged

[DI] Add ContainerBuilder::fileExists() for checking/tracking resource existence #21408

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
4 changes: 1 addition & 3 deletions src/Symfony/Bridge/Doctrine/DependencyInjection/CompilerPass/DoctrineValidationPass.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\Config\Resource\FileResource;

/**
* Registers additional validators.
Expand Down Expand Up @@ -60,9 +59,8 @@ private function updateValidatorMappingFiles(ContainerBuilder $container, $mappi

foreach ($container->getParameter('kernel.bundles') as $bundle) {
$reflection = new \ReflectionClass($bundle);
if (is_file($file = dirname($reflection->getFileName()).'/'.$validationPath)) {
if ($container->fileExists($file = dirname($reflection->getFileName()).'/'.$validationPath)) {
$files[] = $file;
$container->addResource(new FileResource($file));
}
}

Expand Down
32 changes: 10 additions & 22 deletions src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
use Symfony\Component\Config\Resource\FileResource;
use Symfony\Component\Config\Resource\DirectoryResource;
use Symfony\Component\Finder\Finder;
use Symfony\Component\HttpKernel\DependencyInjection\Extension;
use Symfony\Component\Config\FileLocator;
Expand Down Expand Up @@ -875,32 +874,28 @@ private function registerTranslatorConfiguration(array $config, ContainerBuilder
}
$rootDir = $container->getParameter('kernel.root_dir');
foreach ($container->getParameter('kernel.bundles_metadata') as $name => $bundle) {
if (is_dir($dir = $bundle['path'].'/Resources/translations')) {
if ($container->fileExists($dir = $bundle['path'].'/Resources/translations')) {
$dirs[] = $dir;
}
if (is_dir($dir = $rootDir.sprintf('/Resources/%s/translations', $name))) {
if ($container->fileExists($dir = $rootDir.sprintf('/Resources/%s/translations', $name))) {
$dirs[] = $dir;
}
}

foreach ($config['paths'] as $dir) {
if (is_dir($dir)) {
if ($container->fileExists($dir)) {
$dirs[] = $dir;
} else {
throw new \UnexpectedValueException(sprintf('%s defined in translator.paths does not exist or is not a directory', $dir));
}
}

if (is_dir($dir = $rootDir.'/Resources/translations')) {
if ($container->fileExists($dir = $rootDir.'/Resources/translations')) {
$dirs[] = $dir;
}

// Register translation resources
if ($dirs) {
foreach ($dirs as $dir) {
$container->addResource(new DirectoryResource($dir));
}

$files = array();
$finder = Finder::create()
->followLinks()
Expand Down Expand Up @@ -1006,19 +1001,16 @@ private function getValidatorMappingFiles(ContainerBuilder $container, array &$f
foreach ($container->getParameter('kernel.bundles_metadata') as $bundle) {
$dirname = $bundle['path'];

if (is_file($file = $dirname.'/Resources/config/validation.yml')) {
if ($container->fileExists($file = $dirname.'/Resources/config/validation.yml', false)) {
Copy link
Member

Choose a reason for hiding this comment

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

Why did you pass false here (same for similar cases below)?

Copy link
Member Author

Choose a reason for hiding this comment

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

These files do not impact the container (see #21408 (comment))

Copy link
Member

Choose a reason for hiding this comment

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

Ah yeah, just found the hidden discussion too.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should open a PR for older branches to make use of the FileExistenceResource there.

$files['yml'][] = $file;
$container->addResource(new FileResource($file));
}

if (is_file($file = $dirname.'/Resources/config/validation.xml')) {
if ($container->fileExists($file = $dirname.'/Resources/config/validation.xml', false)) {
$files['xml'][] = $file;
$container->addResource(new FileResource($file));
}

if (is_dir($dir = $dirname.'/Resources/config/validation')) {
if ($container->fileExists($dir = $dirname.'/Resources/config/validation')) {
$this->getValidatorMappingFilesFromDir($dir, $files);
$container->addResource(new DirectoryResource($dir));
}
}
}
Expand Down Expand Up @@ -1202,23 +1194,21 @@ private function registerSerializerConfiguration(array $config, ContainerBuilder
foreach ($container->getParameter('kernel.bundles_metadata') as $bundle) {
$dirname = $bundle['path'];

if (is_file($file = $dirname.'/Resources/config/serialization.xml')) {
if ($container->fileExists($file = $dirname.'/Resources/config/serialization.xml', false)) {
$definition = new Definition('Symfony\Component\Serializer\Mapping\Loader\XmlFileLoader', array($file));
$definition->setPublic(false);

$serializerLoaders[] = $definition;
$container->addResource(new FileResource($file));
}

if (is_file($file = $dirname.'/Resources/config/serialization.yml')) {
if ($container->fileExists($file = $dirname.'/Resources/config/serialization.yml', false)) {
$definition = new Definition('Symfony\Component\Serializer\Mapping\Loader\YamlFileLoader', array($file));
$definition->setPublic(false);

$serializerLoaders[] = $definition;
$container->addResource(new FileResource($file));
}

if (is_dir($dir = $dirname.'/Resources/config/serialization')) {
if ($container->fileExists($dir = $dirname.'/Resources/config/serialization')) {
foreach (Finder::create()->followLinks()->files()->in($dir)->name('*.xml') as $file) {
$definition = new Definition('Symfony\Component\Serializer\Mapping\Loader\XmlFileLoader', array($file->getPathname()));
$definition->setPublic(false);
Expand All @@ -1231,8 +1221,6 @@ private function registerSerializerConfiguration(array $config, ContainerBuilder

$serializerLoaders[] = $definition;
}

$container->addResource(new DirectoryResource($dir));
}
}

Expand Down
10 changes: 3 additions & 7 deletions src/Symfony/Bundle/TwigBundle/DependencyInjection/TwigExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
namespace Symfony\Bundle\TwigBundle\DependencyInjection;

use Symfony\Component\Config\FileLocator;
use Symfony\Component\Config\Resource\FileExistenceResource;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
Expand Down Expand Up @@ -108,10 +107,9 @@ public function load(array $configs, ContainerBuilder $container)
}
}

if (is_dir($dir = $container->getParameter('kernel.root_dir').'/Resources/views')) {
if ($container->fileExists($dir = $container->getParameter('kernel.root_dir').'/Resources/views', false)) {
$twigFilesystemLoaderDefinition->addMethodCall('addPath', array($dir));
}
$container->addResource(new FileExistenceResource($dir));

if (!empty($config['globals'])) {
$def = $container->getDefinition('twig');
Expand Down Expand Up @@ -164,15 +162,13 @@ private function getBundleHierarchy(ContainerBuilder $container)
);
}

if (is_dir($dir = $container->getParameter('kernel.root_dir').'/Resources/'.$name.'/views')) {
if ($container->fileExists($dir = $container->getParameter('kernel.root_dir').'/Resources/'.$name.'/views', false)) {
$bundleHierarchy[$name]['paths'][] = $dir;
}
$container->addResource(new FileExistenceResource($dir));

if (is_dir($dir = $bundle['path'].'/Resources/views')) {
if ($container->fileExists($dir = $bundle['path'].'/Resources/views', false)) {
$bundleHierarchy[$name]['paths'][] = $dir;
}
$container->addResource(new FileExistenceResource($dir));

if (null === $bundle['parent']) {
continue;
Expand Down
5 changes: 4 additions & 1 deletion src/Symfony/Bundle/TwigBundle/composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
"require-dev": {
"symfony/asset": "~2.8|~3.0",
"symfony/stopwatch": "~2.8|~3.0",
"symfony/dependency-injection": "~2.8|~3.0",
"symfony/dependency-injection": "~3.3",
"symfony/expression-language": "~2.8|~3.0",
"symfony/finder": "~2.8|~3.0",
"symfony/form": "~2.8|~3.0",
Expand All @@ -36,6 +36,9 @@
"symfony/framework-bundle": "^3.2.2",
"doctrine/annotations": "~1.0"
},
"conflict": {
"symfony/dependency-injection": "<3.3"
},
"autoload": {
"psr-4": { "Symfony\\Bundle\\TwigBundle\\": "" },
"exclude-from-classmap": [
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Component/DependencyInjection/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ CHANGELOG
3.3.0
-----

* added `ContainerBuilder::fileExists()` for checking and tracking file or directory existence
* deprecated autowiring-types, use aliases instead
* [EXPERIMENTAL] added support for getter-injection
* added support for omitting the factory class name in a service definition if the definition class is set
Expand Down
38 changes: 38 additions & 0 deletions src/Symfony/Component/DependencyInjection/ContainerBuilder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException;
use Symfony\Component\DependencyInjection\Extension\ExtensionInterface;
use Symfony\Component\DependencyInjection\ParameterBag\EnvPlaceholderParameterBag;
use Symfony\Component\Config\Resource\DirectoryResource;
use Symfony\Component\Config\Resource\FileExistenceResource;
use Symfony\Component\Config\Resource\FileResource;
use Symfony\Component\Config\Resource\ResourceInterface;
use Symfony\Component\DependencyInjection\LazyProxy\Instantiator\InstantiatorInterface;
Expand Down Expand Up @@ -1321,6 +1323,42 @@ public static function getServiceConditionals($value)
return $services;
}

/**
* Checks whether the requested file or directory exists and registers the result for resource tracking.
*
* @param string $path The file or directory path for which to check the existence
* @param bool|string $trackContents Whether to track contents of the given resource. If a string is passed,
* it will be used as pattern for tracking contents of the requested directory
*
* @return bool
*
* @final
*/
public function fileExists($path, $trackContents = true)
Copy link
Contributor

Choose a reason for hiding this comment

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

I know I'm late to the party, but this name doesn't suggest any side effects. The method actually adds resources and could be better named to reflect this fact.

Copy link
Member Author

Choose a reason for hiding this comment

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

@jakzal This follows up #21452 (comment) which gives the arguments against the need for reflecting the tracking effect in the method name.

{
$exists = file_exists($path);

if (!$this->trackResources) {
return $exists;
}

if (!$exists) {
$this->addResource(new FileExistenceResource($path));

return $exists;
}

if ($trackContents) {
if (is_file($path)) {
$this->addResource(new FileResource($path));
} else {
$this->addResource(new DirectoryResource($path, is_string($trackContents) ? $trackContents : null));
}
}

return $exists;
}

/**
* Retrieves the currently set proxy instantiator or instantiates one.
*
Expand Down
20 changes: 20 additions & 0 deletions src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
require_once __DIR__.'/Fixtures/includes/ProjectExtension.php';

use Symfony\Component\Config\Resource\ResourceInterface;
use Symfony\Component\Config\Resource\DirectoryResource;
use Symfony\Component\DependencyInjection\Alias;
use Symfony\Component\DependencyInjection\Argument\ClosureProxyArgument;
use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
Expand Down Expand Up @@ -682,6 +683,25 @@ public function testResources()
$this->assertEquals(array(), $container->getResources());
}

public function testFileExists()
{
$container = new ContainerBuilder();
$a = new FileResource(__DIR__.'/Fixtures/xml/services1.xml');
$b = new FileResource(__DIR__.'/Fixtures/xml/services2.xml');
$c = new DirectoryResource($dir = dirname($b));

$this->assertTrue($container->fileExists((string) $a) &&a 61AD mp; $container->fileExists((string) $b) && $container->fileExists($dir));

$resources = array();
foreach ($container->getResources() as $resource) {
if (false === strpos($resource, '.php')) {
$resources[] = $resource;
}
}

$this->assertEquals(array($a, $b, $c), $resources, '->getResources() returns an array of resources read for the current configuration');
}

public function testExtension()
{
$container = new ContainerBuilder();
Expand Down
0