Skip to content

Commit

Permalink
feature #33258 [HttpKernel] deprecate global dir to load resources fr…
Browse files Browse the repository at this point in the history
…om (Tobion)

This PR was merged into the 4.4 branch.

Discussion
----------

[HttpKernel] deprecate global dir to load resources from

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | yes <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | #31915   <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        |

Replaces #31958

Here two example deprecations by adding files in the deprecated locations:
```
Overwriting the resource "@AcmeBundle/Resources/config/routing.yaml" with "/vagrant/src/Resources/AcmeBundle/config/routing.yaml" is deprecated since Symfony 4.4 and will be removed in 5.0.
Loading the file "foobar.yaml" from the global resource directory "/vagrant/src" is deprecated since Symfony 4.4 and will be removed in 5.0.
```

Commits
-------

aa82566 [HttpKernel] deprecate global dir to load resources from
  • Loading branch information
nicolas-grekas committed Aug 21, 2019
2 parents 2984ab7 + aa82566 commit f499083
Show file tree
Hide file tree
Showing 9 changed files with 94 additions and 25 deletions.
6 changes: 6 additions & 0 deletions UPGRADE-4.4.md
Expand Up @@ -143,6 +143,12 @@ HttpKernel

As many bundles must be compatible with a range of Symfony versions, the current
directory convention is not deprecated yet, but it will be in the future.
* Deprecated the second and third argument of `KernelInterface::locateResource`
* Deprecated the second and third argument of `FileLocator::__construct`
* Deprecated loading resources from `%kernel.root_dir%/Resources` and `%kernel.root_dir%` as
fallback directories. Resources like service definitions are usually loaded relative to the
current directory or with a glob pattern. The fallback directories have never been advocated
so you likely do not use those in any app based on the SF Standard or Flex edition.

Lock
----
Expand Down
4 changes: 4 additions & 0 deletions UPGRADE-5.0.md
Expand Up @@ -333,6 +333,10 @@ HttpKernel

As many bundles must be compatible with a range of Symfony versions, the current
directory convention is not deprecated yet, but it will be in the future.
* Removed the second and third argument of `KernelInterface::locateResource`
* Removed the second and third argument of `FileLocator::__construct`
* Removed loading resources from `%kernel.root_dir%/Resources` and `%kernel.root_dir%` as
fallback directories.

Intl
----
Expand Down
Expand Up @@ -91,6 +91,7 @@
<argument type="collection">
<argument>%kernel.root_dir%</argument>
</argument>
<argument>false</argument>
</service>
<service id="Symfony\Component\HttpKernel\Config\FileLocator" alias="file_locator" />

Expand Down
6 changes: 6 additions & 0 deletions src/Symfony/Component/HttpKernel/CHANGELOG.md
Expand Up @@ -6,6 +6,12 @@ CHANGELOG

* The `DebugHandlersListener` class has been marked as `final`
* Added new Bundle directory convention consistent with standard skeletons
* Deprecated the second and third argument of `KernelInterface::locateResource`
* Deprecated the second and third argument of `FileLocator::__construct`
* Deprecated loading resources from `%kernel.root_dir%/Resources` and `%kernel.root_dir%` as
fallback directories. Resources like service definitions are usually loaded relative to the
current directory or with a glob pattern. The fallback directories have never been advocated
so you likely do not use those in any app based on the SF Standard or Flex edition.

4.3.0
-----
Expand Down
51 changes: 42 additions & 9 deletions src/Symfony/Component/HttpKernel/Config/FileLocator.php
Expand Up @@ -22,18 +22,28 @@
class FileLocator extends BaseFileLocator
{
private $kernel;
private $path;

/**
* @param string|null $path The path the global resource directory
* @param array $paths An array of paths where to look for resources
* @deprecated since Symfony 4.4
*/
public function __construct(KernelInterface $kernel, string $path = null, array $paths = [])
private $path;

public function __construct(KernelInterface $kernel/*, string $path = null, array $paths = [], bool $triggerDeprecation = true*/)
{
$this->kernel = $kernel;
if (null !== $path) {
$this->path = $path;
$paths[] = $path;

if (2 <= \func_num_args()) {
$this->path = func_get_arg(1);
$paths = 3 <= \func_num_args() ? func_get_arg(2) : [];
if (null !== $this->path) {
$paths[] = $this->path;
}

if (4 !== \func_num_args() || func_get_arg(3)) {
@trigger_error(sprintf('Passing more than one argument to %s is deprecated since Symfony 4.4 and will be removed in 5.0.', __METHOD__), E_USER_DEPRECATED);
}
} else {
$paths = [];
}

parent::__construct($paths);
Expand All @@ -45,9 +55,32 @@ public function __construct(KernelInterface $kernel, string $path = null, array
public function locate($file, $currentPath = null, $first = true)
{
if (isset($file[0]) && '@' === $file[0]) {
return $this->kernel->locateResource($file, $this->path, $first);
return $this->kernel->locateResource($file, $this->path, $first, false);
}

$locations = parent::locate($file, $currentPath, $first);

if (isset($file[0]) && !(
'/' === $file[0] || '\\' === $file[0]
|| (\strlen($file) > 3 && ctype_alpha($file[0]) && ':' === $file[1] && ('\\' === $file[2] || '/' === $file[2]))
|| null !== parse_url($file, PHP_URL_SCHEME)
)) {
// no need to trigger deprecations when the loaded file is given as absolute path
foreach ($this->paths as $deprecatedPath) {
if (\is_array($locations)) {
foreach ($locations as $location) {
if (0 === strpos($location, $deprecatedPath) && (null === $currentPath || false === strpos($location, $currentPath))) {
@trigger_error(sprintf('Loading the file "%s" from the global resource directory "%s" is deprecated since Symfony 4.4 and will be removed in 5.0.', $file, $deprecatedPath), E_USER_DEPRECATED);
}
}
} else {
if (0 === strpos($locations, $deprecatedPath) && (null === $currentPath || false === strpos($locations, $currentPath))) {
@trigger_error(sprintf('Loading the file "%s" from the global resource directory "%s" is deprecated since Symfony 4.4 and will be removed in 5.0.', $file, $deprecatedPath), E_USER_DEPRECATED);
}
}
}
}

return parent::locate($file, $currentPath, $first);
return $locations;
}
}
19 changes: 16 additions & 3 deletions src/Symfony/Component/HttpKernel/Kernel.php
Expand Up @@ -236,11 +236,21 @@ public function getBundle($name)

/**
* {@inheritdoc}
*
* @throws \RuntimeException if a custom resource is hidden by a resource in a derived bundle
*/
public function locateResource($name, $dir = null, $first = true)
public function locateResource($name/*, $dir = null, $first = true, $triggerDeprecation = true*/)
{
if (2 <= \func_num_args()) {
$dir = func_get_arg(1);
$first = 3 <= \func_num_args() ? func_get_arg(2) : true;

if (4 !== \func_num_args() || func_get_arg(3)) {
@trigger_error(sprintf('Passing more than one argument to %s is deprecated since Symfony 4.4 and will be removed in 5.0.', __METHOD__), E_USER_DEPRECATED);
}
} else {
$dir = null;
$first = true;
}

if ('@' !== $name[0]) {
throw new \InvalidArgumentException(sprintf('A resource name must start with @ ("%s" given).', $name));
}
Expand All @@ -262,6 +272,9 @@ public function locateResource($name, $dir = null, $first = true)

if ($isResource && file_exists($file = $dir.'/'.$bundle->getName().$overridePath)) {
$files[] = $file;

// see https://symfony.com/doc/current/bundles/override.html on how to overwrite parts of a bundle
@trigger_error(sprintf('Overwriting the resource "%s" with "%s" is deprecated since Symfony 4.4 and will be removed in 5.0.', $name, $file), E_USER_DEPRECATED);
}

if (file_exists($file = $bundle->getPath().'/'.$path)) {
Expand Down
15 changes: 3 additions & 12 deletions src/Symfony/Component/HttpKernel/KernelInterface.php
Expand Up @@ -80,23 +80,14 @@ public function getBundle($name);
* where BundleName is the name of the bundle
* and the remaining part is the relative path in the bundle.
*
* If $dir is passed, and the first segment of the path is "Resources",
* this method will look for a file named:
* @param string $name A resource name to locate
*
* $dir/<BundleName>/path/without/Resources
*
* before looking in the bundle resource folder.
*
* @param string $name A resource name to locate
* @param string $dir A directory where to look for the resource first
* @param bool $first Whether to return the first path or paths for all matching bundles
*
* @return string|array The absolute path of the resource or an array if $first is false
* @return string|array The absolute path of the resource or an array if $first is false (array return value is deprecated)
*
* @throws \InvalidArgumentException if the file cannot be found or the name is not valid
* @throws \RuntimeException if the name contains invalid/unsafe characters
*/
public function locateResource($name, $dir = null, $first = true);
public function locateResource($name/*, $dir = null, $first = true*/);

/**
* Gets the name of the kernel.
Expand Down
Expand Up @@ -34,6 +34,9 @@ public function testLocate()
$locator->locate('/some/path');
}

/**
* @group legacy
*/
public function testLocateWithGlobalResourcePath()
{
$kernel = $this->getMockBuilder('Symfony\Component\HttpKernel\KernelInterface')->getMock();
Expand Down
14 changes: 13 additions & 1 deletion src/Symfony/Component/HttpKernel/Tests/KernelTest.php
Expand Up @@ -389,6 +389,9 @@ public function testLocateResourceReturnsTheFirstThatMatches()
$this->assertEquals(__DIR__.'/Fixtures/Bundle1Bundle/foo.txt', $kernel->locateResource('@Bundle1Bundle/foo.txt'));
}

/**
* @group legacy
*/
public function testLocateResourceIgnoresDirOnNonResource()
{
$kernel = $this->getKernel(['getBundle']);
Expand All @@ -404,6 +407,9 @@ public function testLocateResourceIgnoresDirOnNonResource()
);
}

/**
* @group legacy
*/
public function testLocateResourceReturnsTheDirOneForResources()
{
$kernel = $this->getKernel(['getBundle']);
Expand All @@ -419,7 +425,10 @@ public function testLocateResourceReturnsTheDirOneForResources()
);
}

public function testLocateResourceOnDirectories()
/**
* @group legacy
*/
public function testLocateResourceOnDirectoriesWithOverwrite()
{
$kernel = $this->getKernel(['getBundle']);
$kernel
Expand All @@ -436,7 +445,10 @@ public function testLocateResourceOnDirectories()
__DIR__.'/Fixtures/Resources/FooBundle',
$kernel->locateResource('@FooBundle/Resources', __DIR__.'/Fixtures/Resources')
);
}

public function testLocateResourceOnDirectories()
{
$kernel = $this->getKernel(['getBundle']);
$kernel
->expects($this->exactly(2))
Expand Down

0 comments on commit f499083

Please sign in to comment.