8000 [DependencyInjection] deprecate access to private shared services. Fi… · hhamon/symfony@02e1480 · GitHub
[go: up one dir, main page]
Skip to content

Commit 02e1480

Browse files
author
Hugo Hamon
committed
[DependencyInjection] deprecate access to private shared services. Fixes issue symfony#19117.
1 parent 1298ce5 commit 02e1480

File tree

9 files changed

+114
-29
lines changed

9 files changed

+114
-29
lines changed

‎UPGRADE-4.0.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,16 @@ DependencyInjection
1818

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

21+
* Resetting a private shared service from outside of the container with the
22+
`set()` method of the `Container` class is no longer supported and will raise
23+
an exception.
24+
25+
* Checking the existence of a private shared service with the `has()` method of
26+
the `Container` class is no longer supported and will raise an exception.
27+
28+
* Requesting a private shared service with the `get()` method of the `Container`
29+
class is no longer supported and will raise an exception.
30+
2131
Form
2232
----
2333

‎src/Symfony/Bridge/ProxyManager/LazyProxy/PhpDumper/ProxyDumper.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ public function getProxyFactoryCode(Definition $definition, $id)
9090
9191
$instantiation $constructorCall(
9292
function (&\$wrappedInstance, \ProxyManager\Proxy\LazyLoadingInterface \$proxy) {
93-
\$wrappedInstance = \$this->$methodName(false);
93+
\$wrappedInstance = \$this->$methodName(false, false);
9494
9595
\$proxy->setProxyInitializer(null);
9696

‎src/Symfony/Bridge/ProxyManager/Tests/LazyProxy/Fixtures/php/lazy_service_structure.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ class ProjectServiceContainer extends Container
99

1010
return $this->services['foo'] =%sstdClass_%s(
1111
function (&$wrappedInstance, \ProxyManager\Proxy\LazyLoadingInterface $proxy) {
12-
$wrappedInstance = $this->getFooService(false);
12+
$wrappedInstance = $this->getFooService(false, false);
1313

1414
$proxy->setProxyInitializer(null);
1515

‎src/Symfony/Bridge/ProxyManager/Tests/LazyProxy/PhpDumper/ProxyDumperTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public function testGetProxyFactoryCodeWithCustomMethod()
7272
'%wif ($lazyLoad) {%wreturn $this->services[\'foo\'] =%s'
7373
.'SymfonyBridgeProxyManagerTestsLazyProxyPhpDumperProxyDumperTest_%s(%wfunction '
7474
.'(&$wrappedInstance, \ProxyManager\Proxy\LazyLoadingInterface $proxy) {'
75-
.'%w$wrappedInstance = $this->getFoo2Service(false);%w$proxy->setProxyInitializer(null);'
75+
.'%w$wrappedInstance = $this->getFoo2Service(false, false);%w$proxy->setProxyInitializer(null);'
7676
.'%wreturn true;%w}%w);%w}%w',
7777
$code
7878
);
@@ -93,7 +93,7 @@ public function testGetProxyFactoryCode()
9393
'%wif ($lazyLoad) {%wreturn $this->services[\'foo\'] =%s'
9494
.'SymfonyBridgeProxyManagerTestsLazyProxyPhpDumperProxyDumperTest_%s(%wfunction '
9595
.'(&$wrappedInstance, \ProxyManager\Proxy\LazyLoadingInterface $proxy) {'
96-
.'%w$wrappedInstance = $this->getFooService(false);%w$proxy->setProxyInitializer(null);'
96+
.'%w$wrappedInstance = $this->getFooService(false, false);%w$proxy->setProxyInitializer(null);'
9797
.'%wreturn true;%w}%w);%w}%w',
9898
$code
9999
);

‎src/Symfony/Component/DependencyInjection/Container.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ class Container implements ResettableContainerInterface
6565

6666
protected $services = array();
6767
protected $methodMap = array();
68+
protected $privates = array();
6869
protected $aliases = array();
6970
protected $loading = array();
7071

@@ -176,6 +177,10 @@ public function set($id, $service)
176177
if (null === $service) {
177178
unset($this->services[$id]);
178179
}
180+
181+
if (isset($this->privates[$id])) {
182+
@trigger_error(sprintf('Resetting the private service "%s" from outside of the container is deprecated since Symfony 3.2 and won\'t be supported anymore in Symfony 4.0. An exception will be thrown instead in Symfony 4.0.', $id), E_USER_DEPRECATED);
183+
}
179184
}
180185

181186
/**
@@ -198,6 +203,10 @@ public function has($id)
198203
if (--$i && $id !== $lcId = strtolower($id)) {
199204
$id = $lcId;
200205
} else {
206+
if (isset($this->privates[$id])) {
207+
@trigger_error(sprintf('Checking for the existence of the private service "%s" is deprecated since Symfony 3.2 and won\'t be supported anymore in Symfony 4.0. An exception will be thrown instead in Symfony 4.0.', $id), E_USER_DEPRECATED);
208+
}
209+
201210
return method_exists($this, 'get'.strtr($id, $this->underscoreMap).'Service');
202211
}
203212
}
@@ -268,6 +277,9 @@ public function get($id, $invalidBehavior = self::EXCEPTION_ON_INVALID_REFERENCE
268277

269278
return;
270279
}
280+
if (isset($this->privates[$id])) {
281+
@trigger_error(sprintf('Requesting the shared private service "%s" is deprecated since Symfony 3.2 and won\'t be supported anymore in Symfony 4.0. An exception will be thrown instead in Symfony 4.0.', $id), E_USER_DEPRECATED);
282+
}
271283

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

‎src/Symfony/Component/DependencyInjection/ContainerBuilder.php

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -559,11 +559,12 @@ public function compile()
559559
$compiler->compile($this);
560560
$this->compiled = true;
561561

562-
if ($this->trackResources) {
563-
foreach ($this->definitions as $definition) {
564-
if ($definition->isLazy() && ($class = $definition->getClass()) && class_exists($class)) {
565-
$this->addClassResource(new \ReflectionClass($class));
566-
}
562+
foreach ($this->definitions as $id => $definition) {
563+
if (!$definition->isPublic()) {
564+
$this->privates[$id] = true;
565+
}
566+
if ($this->trackResources && $definition->isLazy() && ($class = $definition->getClass()) && class_exists($class)) {
567+
$this->addClassResource(new \ReflectionClass($class));
567568
}
568569
}
569570

‎src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php

Lines changed: 37 additions & 7 deletions
639
Original file line numberDiff line numberDiff line change
@@ -566,6 +566,7 @@ private function addService($id, $definition)
566566
$this->referenceVariables = array();
567567
$this->variableCount = 0;
568568

569+
$args = array();
569570
$return = array();
570571

571572
if ($definition->isSynthetic()) {
@@ -606,12 +607,11 @@ private function addService($id, $definition)
606607
}
607608

608609
if (!$definition->isPublic()) {
610+
$args[] = '$triggerDeprecation = true';
609611
$doc .= <<<'EOF'
610612
611613
*
612614
* This service is private.
613-
* If you want to be able to request this service from the container directly,
614-
* make it public, otherwise you might end up with broken code.
615615
EOF;
616616
}
617617

@@ -623,17 +623,16 @@ private function addService($id, $definition)
623623
EOF;
624624
}
625625

626+
$args = array();
626627
if ($definition->isLazy()) {
627-
$lazyInitialization = '$lazyLoad = true';
628+
$args[] = '$lazyLoad = true';
628629
$lazyInitializationDoc = "\n * @param bool \$lazyLoad whether to try lazy-loading the service with a proxy\n *";
629630
} else {
630-
$lazyInitialization = '';
631631
$lazyInitializationDoc = '';
632632
}
633633

634-
// with proxies, for 5.3.3 compatibility, the getter must be public to be accessible to the initializer
634+
$args = implode(', ', $args);
635635
$isProxyCandidate = $this->getProxyDumper()->isProxyCandidate($definition);
636-
$visibility = $isProxyCandidate ? 'public' : 'protected';
637636
$methodName = $this->generateMethodName($id);
638637
$code = <<<EOF
638
@@ -642,7 +641,7 @@ private function addService($id, $definition)
642641
*$lazyInitializationDoc
643642
* $return
644643
*/
645-
{$visibility} function {$methodName}($lazyInitialization)
644+
protected function {$methodName}($args)
646645
{
647646
648647
EOF;
@@ -804,6 +803,7 @@ public function __construct()
804803
EOF;
805804

806805
$code .= $this->addMethodMap();
806+
$code .= $this->addPrivateServices();
807807
$code .= $this->addAliases();
808808

809809
$code .= <<<'EOF'
@@ -888,6 +888,36 @@ private function addMethodMap()
888888
return $code." );\n";
889889
}
890890

891+
/**
892+
* Adds the $privates property definition.
893+
*
894+
* @return string
895+
*/
896+
private function addPrivateServices()
897+
{
898+
if (!$definitions = $this->container->getDefinitions()) {
899+
return '';
900+
}
901+
902+
$code = '';
903+
ksort($definitions);
904+
foreach ($definitions as $id => $definition) {
905+
if (!$definition->isPublic()) {
906+
$code .= ' '.var_export($id, true)." => true,\n";
907+
}
908+
}
909+
910+
if (empty($code)) {
911+
return '';
912+
}
913+
914+
$out = " \$this->privates = array(\n";
915+
$out .= $code;
916+
$out .= " );\n";
917+
918+
return $out;
919+
}
920+
891921
/**
892922
* Adds the aliases property definition.
893923
*

‎src/Symfony/Component/DependencyInjection/Tests/ContainerTest.php

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ public function testGetServiceIds()
125125

126126
$sc = new ProjectServiceContainer();
127127
$sc->set('foo', $obj = new \stdClass());
128-
$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()');
128+
$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()');
129129
}
130130

131131
public function testSet()
@@ -306,11 +306,39 @@ public function testThatCloningIsNotSupported()
306306
$this->assertFalse($class->isCloneable());
307307
$this->assertTrue($clone->isPrivate());
308308
}
309+
310+
/** @group legacy */
311+
public function testUnsetInternalPrivateServiceIsDeprecated()
312+
{
313+
$c = new ProjectServiceContainer();
314+
$c->set('internal', null);
315+
}
316+
317+
/** @group legacy */
318+
public function testChangeInternalPrivateServiceIsDeprecated()
319+
{
320+
$c = new ProjectServiceContainer();
321+
$c->set('internal', new \stdClass());
322+
}
323+
324+
/** @group legacy */
325+
public function testCheckExistenceOfAnInternalPrivateServiceIsDeprecated()
326+
{
327+
$c = new ProjectServiceContainer();
328+
$c->has('internal');
329+
}
330+
331+
/** @group legacy */
332+
public function testRequestAnInternalSharedPrivateServiceIsDeprecated()
333+
{
334+
$c = new ProjectServiceContainer();
335+
$c->get('internal');
336+
}
309337
}
310338

311339
class ProjectServiceContainer extends Container
312340
{
313-
public $__bar, $__foo_bar, $__foo_baz;
341+
public $__bar, $__foo_bar, $__foo_baz, $__internal;
314342

315343
public function __construct()
316344
{
@@ -319,9 +347,16 @@ public function __construct()
319347
$this->__bar = new \stdClass();
320348
$this->__foo_bar = new \stdClass();
321349
$this->__foo_baz = new \stdClass();
350+
$this->__internal = new \stdClass();
351+
$this->privates = array('internal' => true);
322352
$this->aliases = array('alias' => 'bar');
323353
}
324354

355+
protected function getInternalService()
356+
{
357+
return $this->__internal;
358+
}
359+
325360
protected function getBarService()
326361
{
327362
return $this->__bar;

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,13 @@ public function __construct()
4949
'request' => 'getRequestService',
5050
'service_from_static_method' => 'getServiceFromStaticMethodService',
5151
);
52+
$this->privates = array(
53+
'configurator_service' => true,
54+
'configurator_service_simple' => true,
55+
'factory_simple' => true,
56+
'inlined' => true,
57+
'new_factory' => true,
58+
);
5259
$this->aliases = array(
5360
'alias_for_alias' => 'foo',
5461
'alias_for_foo' => 'foo',
@@ -299,7 +306,7 @@ protected function getMethodCall1Service()
299306
if ($this->has('foobaz')) {
300307
$instance->setBar($this->get('foobaz', ContainerInterface::NULL_ON_INVALID_REFERENCE));
301308
}
302-
$instance->setBar(($this->get("foo")->foo() . (($this->hasparameter("foo")) ? ($this->getParameter("foo")) : ("default"))));
309+
$instance->setBar(($this->get('foo')->foo().(($this->hasparameter('foo')) ? ($this->getParameter('foo')) : ('default'))));
303310

304311
return $instance;
305312
}
@@ -354,8 +361,6 @@ protected function getServiceFromStaticMethodService()
354361
* This method always returns the same instance of the service.
355362
*
356363
* This service is private.
357-
* If you want to be able to request this service from the container directly,
358-
* make it public, otherwise you might end up with broken code.
359364
*
360365
* @return \ConfClass A ConfClass instance.
361366
*/
@@ -375,8 +380,6 @@ protected function getConfiguratorServiceService()
375380
* This method always returns the same instance of the service.
376381
*
377382
* This service is private.
378-
* If you want to be able to request this service from the container directly,
379-
* make it public, otherwise you might end up with broken code.
380383
*
381384
* @return \ConfClass A ConfClass instance.
382385
*/
@@ -392,8 +395,6 @@ protected function getConfiguratorServiceSimpleService()
392395
* This method always returns the same instance of the service.
393396
*
394397
* This service is private.
395-
* If you want to be able to request this service from the container directly,
396-
* make it public, otherwise you might end up with broken code.
397398
*
398399
* @return \SimpleFactoryClass A SimpleFactoryClass instance.
399400
*/
@@ -409,8 +410,6 @@ protected function getFactorySimpleService()
409410
* This method always returns the same instance of the service.
410411
*
411412
* This service is private.
412-
* If you want to be able to request this service from the container directly,
413-
* make it public, otherwise you might end up with broken code.
414413
*
415414
* @return \Bar A Bar instance.
416415
*/
@@ -431,8 +430,6 @@ protected function getInlinedService()
431430
* This method always returns the same instance of the service.
432431
*
433432
* This service is private.
434-
* If you want to be able to request this service from the container directly,
435-
* make it public, otherwise you might end up with broken code.
436433
*
437434
* @return \FactoryClass A FactoryClass instance.
438435
*/

0 commit comments

Comments
 (0)
0