Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
feature #19146 [DependencyInjection] deprecate access to private shar…
…ed services. (hhamon)

This PR was merged into the 3.2-dev branch.

Discussion
----------

[DependencyInjection] deprecate access to private shared services.

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | #19117
| License       | MIT
| Doc PR        | ~

Commits
-------

4ed70c4 [DependencyInjection] deprecate access to private shared services. Fixes issue #19117.
  • Loading branch information
fabpot committed Jun 29, 2016
2 parents 0eae5d2 + 4ed70c4 commit ed6267d
Show file tree
Hide file tree
Showing 7 changed files with 134 additions and 6 deletions.
9 changes: 9 additions & 0 deletions UPGRADE-4.0.md
Expand Up @@ -18,6 +18,15 @@ DependencyInjection

* Using unsupported options to configure service aliases raises an exception.

* Setting or unsetting a private service with the `Container::set()` method is
no longer supported. Only public services can be set or unset.

* Checking the existence of a private service with the `Container::has()`
method is no longer supported and will return `false`.

* Requesting a private service with the `Container::get()` method is no longer
supported.

Form
----

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

* allowed to prioritize compiler passes by introducing a third argument to `PassConfig::addPass()`, to `Compiler::addPass` and to `ContainerBuilder::addCompilerPass()`
* added support for PHP constants in YAML configuration files
* deprecated the ability to set or unset a private service with the `Container::set()` method
* deprecated the ability to check for the existence of a private service with the `Container::has()` method
* deprecated the ability to request a private service with the `Container::get()` method

3.0.0
-----
Expand Down
16 changes: 16 additions & 0 deletions src/Symfony/Component/DependencyInjection/Container.php
Expand Up @@ -65,6 +65,7 @@ class Container implements ResettableContainerInterface

protected $services = array();
protected $methodMap = array();
protected $privates = array();
protected $aliases = array();
protected $loading = array();

Expand Down Expand Up @@ -176,6 +177,14 @@ public function set($id, $service)
if (null === $service) {
unset($this->services[$id]);
}

if (isset($this->privates[$id])) {
if (null === $service) {
@trigger_error(sprintf('Unsetting the "%s" private service is deprecated since Symfony 3.2 and won\'t be supported anymore in Symfony 4.0.', $id), E_USER_DEPRECATED);
} else {
@trigger_error(sprintf('Setting the "%s" private service is deprecated since Symfony 3.2 and won\'t be supported anymore in Symfony 4.0. A new public service will be created instead.', $id), E_USER_DEPRECATED);
}
}
}

/**
Expand All @@ -198,6 +207,10 @@ public function has($id)
if (--$i && $id !== $lcId = strtolower($id)) {
$id = $lcId;
} else {
if (isset($this->privates[$id])) {
@trigger_error(sprintf('Checking for the existence of the "%s" private service is deprecated since Symfony 3.2 and won\'t be supported anymore in Symfony 4.0.', $id), E_USER_DEPRECATED);
}

return method_exists($this, 'get'.strtr($id, $this->underscoreMap).'Service');
}
}
Expand Down Expand Up @@ -268,6 +281,9 @@ public function get($id, $invalidBehavior = self::EXCEPTION_ON_INVALID_REFERENCE

return;
}
if (isset($this->privates[$id])) {
@trigger_error(sprintf('Requesting the "%s" private service is deprecated since Symfony 3.2 and won\'t be supported anymore in Symfony 4.0.', $id), E_USER_DEPRECATED);
}

$this->loading[$id] = true;

Expand Down
11 changes: 6 additions & 5 deletions src/Symfony/Component/DependencyInjection/ContainerBuilder.php
Expand Up @@ -559,11 +559,12 @@ public function compile()
$compiler->compile($this);
$this->compiled = true;

if ($this->trackResources) {
foreach ($this->definitions as $definition) {
if ($definition->isLazy() && ($class = $definition->getClass()) && class_exists($class)) {
$this->addClassResource(new \ReflectionClass($class));
}
foreach ($this->definitions as $id => $definition) {
if (!$definition->isPublic()) {
$this->privates[$id] = true;
}
if ($this->trackResources && $definition->isLazy() && ($class = $definition->getClass()) && class_exists($class)) {
$this->addClassResource(new \ReflectionClass($class));
}
}

Expand Down
31 changes: 31 additions & 0 deletions src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
Expand Up @@ -804,6 +804,7 @@ public function __construct()
EOF;

$code .= $this->addMethodMap();
$code .= $this->addPrivateServices();
$code .= $this->addAliases();

$code .= <<<'EOF'
Expand Down Expand Up @@ -888,6 +889,36 @@ private function addMethodMap()
return $code." );\n";
}

/**
* Adds the privates property definition.
*
* @return string
*/
private function addPrivateServices()
{
if (!$definitions = $this->container->getDefinitions()) {
return '';
}

$code = '';
ksort($definitions);
foreach ($definitions as $id => $definition) {
if (!$definition->isPublic()) {
$code .= ' '.var_export($id, true)." => true,\n";
}
}

if (empty($code)) {
return '';
}

$out = " \$this->privates = array(\n";
$out .= $code;
$out .= " );\n";

return $out;
}

/**
* Adds the aliases property definition.
*
Expand Down
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Component\DependencyInjection\Tests;

use Symfony\Bridge\PhpUnit\ErrorAssert;
use Symfony\Component\DependencyInjection\Container;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag;
Expand Down Expand Up @@ -125,7 +126,7 @@ public function testGetServiceIds()

$sc = new ProjectServiceContainer();
$sc->set('foo', $obj = new \stdClass());
$this->assertEquals(array('bar', 'foo_bar', 'foo.baz', 'circular', 'throw_exception', 'throws_exception_on_service_configuration', 'service_container', 'foo'), $sc->getServiceIds(), '->getServiceIds() returns defined service ids by getXXXService() methods, followed by service ids defined by set()');
$this->assertEquals(array('internal', 'bar', 'foo_bar', 'foo.baz', 'circular', 'throw_exception', 'throws_exception_on_service_configuration', 'service_container', 'foo'), $sc->getServiceIds(), '->getServiceIds() returns defined service ids by getXXXService() methods, followed by service ids defined by set()');
}

public function testSet()
Expand Down Expand Up @@ -306,13 +307,66 @@ public function testThatCloningIsNotSupported()
$this->assertFalse($class->isCloneable());
$this->assertTrue($clone->isPrivate());
}

/** @group legacy */
public function testUnsetInternalPrivateServiceIsDeprecated()
{
$deprecations = array(
'Unsetting the "internal" private service is deprecated since Symfony 3.2 and won\'t be supported anymore in Symfony 4.0.',
);

ErrorAssert::assertDeprecationsAreTriggered($deprecations, function () {
$c = new ProjectServiceContainer();
$c->set('internal', null);
});
}

/** @group legacy */
public function testChangeInternalPrivateServiceIsDeprecated()
{
$deprecations = array(
'Setting the "internal" private service is deprecated since Symfony 3.2 and won\'t be supported anymore in Symfony 4.0. A new public service will be created instead.',
);

ErrorAssert::assertDeprecationsAreTriggered($deprecations, function () {
$c = new ProjectServiceContainer();
$c->set('internal', new \stdClass());
});
}

/** @group legacy */
public function testCheckExistenceOfAnInternalPrivateServiceIsDeprecated()
{
$deprecations = array(
'Checking for the existence of the "internal" private service is deprecated since Symfony 3.2 and won\'t be supported anymore in Symfony 4.0.',
);

ErrorAssert::assertDeprecationsAreTriggered($deprecations, function () {
$c = new ProjectServiceContainer();
$c->has('internal');
});
}

/** @group legacy */
public function testRequestAnInternalSharedPrivateServiceIsDeprecated()
{
$deprecations = array(
'Requesting the "internal" private service is deprecated since Symfony 3.2 and won\'t be supported anymore in Symfony 4.0.',
);

ErrorAssert::assertDeprecationsAreTriggered($deprecations, function () {
$c = new ProjectServiceContainer();
$c->get('internal');
});
}
}

class ProjectServiceContainer extends Container
{
public $__bar;
public $__foo_bar;
public $__foo_baz;
public $__internal;

public function __construct()
{
Expand All @@ -321,9 +375,16 @@ public function __construct()
$this->__bar = new \stdClass();
$this->__foo_bar = new \stdClass();
$this->__foo_baz = new \stdClass();
$this->__internal = new \stdClass();
$this->privates = array('internal' => true);
$this->aliases = array('alias' => 'bar');
}

protected function getInternalService()
{
return $this->__internal;
}

protected function getBarService()
{
return $this->__bar;
Expand Down
Expand Up @@ -49,6 +49,13 @@ public function __construct()
'request' => 'getRequestService',
'service_from_static_method' => 'getServiceFromStaticMethodService',
);
$this->privates = array(
'configurator_service' => true,
'configurator_service_simple' => true,
'factory_simple' => true,
'inlined' => true,
'new_factory' => true,
);
$this->aliases = array(
'alias_for_alias' => 'foo',
'alias_for_foo' => 'foo',
Expand Down

0 comments on commit ed6267d

Please sign in to comment.