Skip to content

Commit 9713ac2

Browse files
fabpotfabpot
committed
feature #18728 deprecate get() for uncompiled container builders (xabbuh)
This PR was squashed before being merged into the 3.2-dev branch (closes #18728). Discussion ---------- deprecate get() for uncompiled container builders | Q | A | ------------- | --- | Branch? | 3.2 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | symfony/symfony#18673 (comment) | License | MIT | Doc PR | Commits ------- f2e35cb deprecate get() for uncompiled container builders
2 parents a7908d7 + fc3f0e6 commit 9713ac2

File tree

4 files changed

+122
-35
lines changed

4 files changed

+122
-35
lines changed

‎ContainerBuilder.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
9494
*/
9595
private $usedTags = array();
9696

97+
private $compiled = false;
98+
9799
/**
98100
* Sets the track resources flag.
99101
*
@@ -409,6 +411,10 @@ public function has($id)
409411
*/
410412
public function get($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE)
411413
{
414+
if (!$this->compiled) {
415+
@trigger_error(sprintf('Calling %s() before compiling the container is deprecated since version 3.2 and will throw an exception in 4.0.', __METHOD__), E_USER_DEPRECATED);
416+
}
417+
412418
$id = strtolower($id);
413419

414420
if ($service = parent::get($id, ContainerInterface::NULL_ON_INVALID_REFERENCE)) {
@@ -543,6 +549,7 @@ public function compile()
543549
}
544550

545551
$compiler->compile($this);
552+
$this->compiled = true;
546553

547554
if ($this->trackResources) {
548555
foreach ($this->definitions as $definition) {

‎Dumper/GraphvizDumper.php‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -180,14 +180,17 @@ private function findNodes()
180180
}
181181

182182
foreach ($container->getServiceIds() as $id) {
183-
$service = $container->get($id);
184-
185183
if (array_key_exists($id, $container->getAliases())) {
186184
continue;
187185
}
188186

189187
if (!$container->hasDefinition($id)) {
190-
$class = ('service_container' === $id) ? get_class($this->container) : get_class($service);
188+
if ('service_container' === $id) {
189+
$class = get_class($this->container);
190+
} else {
191+
$class = get_class($container->get($id));
192+
}
193+
191194
$nodes[$id] = array('class' => str_replace('\\', '\\\\', $class), 'attributes' => $this->options['node.instance']);
192195
}
193196
}

‎Tests/Compiler/AutoAliasServicePassTest.php‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ public function testProcessWithNonExistingAlias()
5757
$pass->process($container);
5858

5959
$this->assertEquals('Symfony\Component\DependencyInjection\Tests\Compiler\ServiceClassDefault', $container->getDefinition('example')->getClass());
60-
$this->assertInstanceOf('Symfony\Component\DependencyInjection\Tests\Compiler\ServiceClassDefault', $container->get('example'));
6160
}
6261

6362
public function testProcessWithExistingAlias()
@@ -75,7 +74,7 @@ public function testProcessWithExistingAlias()
7574

7675
$this->assertTrue($container->hasAlias('example'));
7776
$this->assertEquals('mysql.example', $container->getAlias('example'));
78-
$this->assertInstanceOf('Symfony\Component\DependencyInjection\Tests\Compiler\ServiceClassMysql', $container->get('example'));
77+
$this->assertSame('Symfony\Component\DependencyInjection\Tests\Compiler\ServiceClassMysql', $container->getDefinition('mysql.example')->getClass());
7978
}
8079

8180
public function testProcessWithManualAlias()
@@ -86,7 +85,7 @@ public function testProcessWithManualAlias()
8685
->addTag('auto_alias', array('format' => '%existing%.example'));
8786

8887
$container->register('mysql.example', 'Symfony\Component\DependencyInjection\Tests\Compiler\ServiceClassMysql');
89-
$container->register('mariadb.example', 'Symfony\Component\DependencyInjection\Tests\Compiler\ServiceClassMariadb');
88+
$container->register('mariadb.example', 'Symfony\Component\DependencyInjection\Tests\Compiler\ServiceClassMariaDb');
9089
$container->setAlias('example', 'mariadb.example');
9190
$container->setParameter('existing', 'mysql');
9291

@@ -95,7 +94,7 @@ public function testProcessWithManualAlias()
9594

9695
$this->assertTrue($container->hasAlias('example'));
9796
$this->assertEquals('mariadb.example', $container->getAlias('example'));
98-
$this->assertInstanceOf('Symfony\Component\DependencyInjection\Tests\Compiler\ServiceClassMariaDb', $container->get('example'));
97+
$this->assertSame('Symfony\Component\DependencyInjection\Tests\Compiler\ServiceClassMariaDb', $container->getDefinition('mariadb.example')->getClass());
9998
}
10099
}
101100

‎Tests/ContainerBuilderTest.php‎

Lines changed: 106 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@
2020
use Symfony\Component\DependencyInjection\ContainerInterface;
2121
use Symfony\Component\DependencyInjection\Definition;
2222
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
23-
use Symfony\Component\DependencyInjection\Exception\InactiveScopeException;
24-
use Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException;
2523
use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException;
2624
use Symfony\Component\DependencyInjection\Loader\ClosureLoader;
2725
use Symfony\Component\DependencyInjection\Reference;
@@ -76,6 +74,9 @@ public function testCreateDeprecatedService()
7674

7775
$builder = new ContainerBuilder();
7876
$builder->setDefinition('deprecated_foo', $definition);
77+
78+
$builder->compile();
79+
7980
$builder->get('deprecated_foo');
8081

8182
restore_error_handler();
@@ -102,41 +103,80 @@ public function testHas()
102103
$this->assertTrue($builder->has('bar'), '->has() returns true if a service exists');
103104
}
104105

105-
public function testGet()
106+
/**
107+
* @expectedException \Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException
108+
* @expectedExceptionMessage You have requested a non-existent service "foo".
109+
*/
110+
public function testGetThrowsExceptionIfServiceDoesNotExist()
106111
{
107112
$builder = new ContainerBuilder();
108-
try {
109-
$builder->get('foo');
110-
$this->fail('->get() throws a ServiceNotFoundException if the service does not exist');
111-
} catch (ServiceNotFoundException $e) {
112-
$this->assertEquals('You have requested a non-existent service "foo".', $e->getMessage(), '->get() throws a ServiceNotFoundException if the service does not exist');
113-
}
113+
$builder->compile();
114+
$builder->get('foo');
115+
}
116+
117+
public function testGetReturnsNullIfServiceDoesNotExistAndInvalidReferenceIsUsed()
118+
{
119+
$builder = new ContainerBuilder();
120+
$builder->compile();
114121

115122
$this->assertNull($builder->get('foo', ContainerInterface::NULL_ON_INVALID_REFERENCE), '->get() returns null if the service does not exist and NULL_ON_INVALID_REFERENCE is passed as a second argument');
123+
}
116124

125+
/**
126+
* @expectedException \Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException
127+
*/
128+
public function testGetThrowsCircularReferenceExceptionIfServiceHasReferenceToItself()
129+
{
130+
$builder = new ContainerBuilder();
131+
$builder->register('baz', 'stdClass')->setArguments(array(new Reference('baz')));
132+
$builder->compile();
133+
$builder->get('baz');
134+
}
135+
136+
public function testGetReturnsSameInstanceWhenServiceIsShared()
137+
{
138+
$builder = new ContainerBuilder();
139+
$builder->register('bar', 'stdClass');
140+
$builder->compile();
141+
142+
$this->assertTrue($builder->get('bar') === $builder->get('bar'), '->get() always returns the same instance if the service is shared');
143+
}
144+
145+
public function testGetCreatesServiceBasedOnDefinition()
146+
{
147+
$builder = new ContainerBuilder();
117148
$builder->register('foo', 'stdClass');
149+
$builder->compile();
150+
118151
$this->assertInternalType('object', $builder->get('foo'), '->get() returns the service definition associated with the id');
152+
}
153+
154+
public function testGetReturnsRegisteredService()
155+
{
156+
$builder = new ContainerBuilder();
119157
$builder->set('bar', $bar = new \stdClass());
120-
$this->assertEquals($bar, $builder->get('bar'), '->get() returns the service associated with the id');
121-
$builder->register('bar', 'stdClass');
122-
$this->assertEquals($bar, $builder->get('bar'), '->get() returns the service associated with the id even if a definition has been defined');
158+
$builder->compile();
123159

124-
$builder->register('baz', 'stdClass')->setArguments(array(new Reference('baz')));
125-
try {
126-
@$builder->get('baz');
127-
$this->fail('->get() throws a ServiceCircularReferenceException if the service has a circular reference to itself');
128-
} catch (\Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException $e) {
129-
$this->assertEquals('Circular reference detected for service "baz", path: "baz".', $e->getMessage(), '->get() throws a LogicException if the service has a circular reference to itself');
130-
}
160+
$this->assertSame($bar, $builder->get('bar'), '->get() returns the service associated with the id');
161+
}
131162

132-
$this->assertTrue($builder->get('bar') === $builder->get('bar'), '->get() always returns the same instance if the service is shared');
163+
public function testRegisterDoesNotOverrideExistingService()
164+
{
165+
$builder = new ContainerBuilder();
166+
$builder->set('bar', $bar = new \stdClass());
167+
$builder->register('bar', 'stdClass');
168+
$builder->compile();
169+
170+
$this->assertSame($bar, $builder->get('bar'), '->get() returns the service associated with the id even if a definition has been defined');
133171
}
134172

135173
public function testNonSharedServicesReturnsDifferentInstances()
136174
{
137175
$builder = new ContainerBuilder();
138176
$builder->register('bar', 'stdClass')->setShared(false);
139177

178+
$builder->compile();
179+
140180
$this->assertNotSame($builder->get('bar'), $builder->get('bar'));
141181
}
142182

@@ -149,6 +189,8 @@ public function testGetUnsetLoadingServiceWhenCreateServiceThrowsAnException()
149189
$builder = new ContainerBuilder();
150190
$builder->register('foo', 'stdClass')->setSynthetic(true);
151191

192+
$builder->compile();
193+
152194
// we expect a RuntimeException here as foo is synthetic
153195
try {
154196
$builder->get('foo');
@@ -177,6 +219,9 @@ public function testAliases()
177219
$this->assertFalse($builder->hasAlias('foobar'), '->hasAlias() returns false if the alias does not exist');
178220
$this->assertEquals('foo', (string) $builder->getAlias('bar'), '->getAlias() returns the aliased service');
179221
$this->assertTrue($builder->has('bar'), '->setAlias() defines a new service');
222+
223+
$builder->compile();
224+
180225
$this->assertTrue($builder->get('bar') === $builder->get('foo'), '->setAlias() creates a service that is an alias to another one');
181226

182227
try {
@@ -244,6 +289,9 @@ public function testSetReplacesAlias()
244289
$builder->set('aliased', new \stdClass());
245290

246291
$builder->set('alias', $foo = new \stdClass());
292+
293+
$builder->compile();
294+
247295
$this->assertSame($foo, $builder->get('alias'), '->set() replaces an existing alias');
248296
}
249297

@@ -261,9 +309,12 @@ public function testCreateService()
261309
{
262310
$builder = new ContainerBuilder();
263311
$builder->register('foo1', 'Bar\FooClass')->setFile(__DIR__.'/Fixtures/includes/foo.php');
264-
$this->assertInstanceOf('\Bar\FooClass', $builder->get('foo1'), '->createService() requires the file defined by the service definition');
265312
$builder->register('foo2', 'Bar\FooClass')->setFile(__DIR__.'/Fixtures/includes/%file%.php');
266313
$builder->setParameter('file', 'foo');
314+
315+
$builder->compile();
316+
317+
$this->assertInstanceOf('\Bar\FooClass', $builder->get('foo1'), '->createService() requires the file defined by the service definition');
267318
$this->assertInstanceOf('\Bar\FooClass', $builder->get('foo2'), '->createService() replaces parameters in the file provided by the service definition');
268319
}
269320

@@ -274,6 +325,8 @@ public function testCreateProxyWithRealServiceInstantiator()
274325
$builder->register('foo1', 'Bar\FooClass')->setFile(__DIR__.'/Fixtures/includes/foo.php');
275326
$builder->getDefinition('foo1')->setLazy(true);
276327

328+
$builder->compile();
329+
277330
$foo1 = $builder->get('foo1');
278331

279332
$this->assertSame($foo1, $builder->get('foo1'), 'The same proxy is retrieved on multiple subsequent calls');
@@ -285,6 +338,9 @@ public function testCreateServiceClass()
285338
$builder = new ContainerBuilder();
286339
$builder->register('foo1', '%class%');
287340
$builder->setParameter('class', 'stdClass');
341+
342+
$builder->compile();
343+
288344
$this->assertInstanceOf('\stdClass', $builder->get('foo1'), '->createService() replaces parameters in the class provided by the service definition');
289345
}
290346

@@ -294,6 +350,9 @@ public function testCreateServiceArguments()
294350
$builder->register('bar', 'stdClass');
295351
$builder->register('foo1', 'Bar\FooClass')->addArgument(array('foo' => '%value%', '%value%' => 'foo', new Reference('bar'), '%%unescape_it%%'));
296352
$builder->setParameter('value', 'bar');
353+
354+
$builder->compile();
355+
297356
$this->assertEquals(array('foo' => 'bar', 'bar' => 'foo', $builder->get('bar'), '%unescape_it%'), $builder->get('foo1')->arguments, '->createService() replaces parameters and service references in the arguments provided by the service definition');
298357
}
299358

@@ -305,6 +364,8 @@ public function testCreateServiceFactory()
305364
$builder->register('bar', 'Bar\FooClass')->setFactory(array(new Definition('Bar\FooClass'), 'getInstance'));
306365
$builder->register('baz', 'Bar\FooClass')->setFactory(array(new Reference('bar'), 'getInstance'));
307366

367+
$builder->compile();
368+
308369
$this->assertTrue($builder->get('foo')->called, '->createService() calls the factory method to create the service instance');
309370
$this->assertTrue($builder->get('qux')->called, '->createService() calls the factory method to create the service instance');
310371
$this->assertTrue($builder->get('bar')->called, '->createService() uses anonymous service as factory');
@@ -317,6 +378,9 @@ public function testCreateServiceMethodCalls()
317378
$builder->register('bar', 'stdClass');
318379
$builder->register('foo1', 'Bar\FooClass')->addMethodCall('setBar', array(array('%value%', new Reference('bar'))));
319380
$builder->setParameter('value', 'bar');
381+
382+
$builder->compile();
383+
320384
$this->assertEquals(array('bar', $builder->get('bar')), $builder->get('foo1')->bar, '->createService() replaces the values in the method calls arguments');
321385
}
322386

@@ -326,6 +390,9 @@ public function testCreateServiceMethodCallsWithEscapedParam()
326390
$builder->register('bar', 'stdClass');
327391
$builder->register('foo1', 'Bar\FooClass')->addMethodCall('setBar', array(array('%%unescape_it%%')));
328392
$builder->setParameter('value', 'bar');
393+
394+
$builder->compile();
395+
329396
$this->assertEquals(array('%unescape_it%'), $builder->get('foo1')->bar, '->createService() replaces the values in the method calls arguments');
330397
}
331398

@@ -335,27 +402,30 @@ public function testCreateServiceProperties()
335402
$builder->register('bar', 'stdClass');
336403
$builder->register('foo1', 'Bar\FooClass')->setProperty('bar', array('%value%', new Reference('bar'), '%%unescape_it%%'));
337404
$builder->setParameter('value', 'bar');
405+
406+
$builder->compile();
407+
338408
$this->assertEquals(array('bar', $builder->get('bar'), '%unescape_it%'), $builder->get('foo1')->bar, '->createService() replaces the values in the properties');
339409
}
340410

341411
public function testCreateServiceConfigurator()
342412
{
343413
$builder = new ContainerBuilder();
344414
$builder->register('foo1', 'Bar\FooClass')->setConfigurator('sc_configure');
345-
$this->assertTrue($builder->get('foo1')->configured, '->createService() calls the configurator');
346-
347415
$builder->register('foo2', 'Bar\FooClass')->setConfigurator(array('%class%', 'configureStatic'));
348416
$builder->setParameter('class', 'BazClass');
349-
$this->assertTrue($builder->get('foo2')->configured, '->createService() calls the configurator');
350-
351417
$builder->register('baz', 'BazClass');
352418
$builder->register('foo3', 'Bar\FooClass')->setConfigurator(array(new Reference('baz'), 'configure'));
353-
$this->assertTrue($builder->get('foo3')->configured, '->createService() calls the configurator');
354-
355419
$builder->register('foo4', 'Bar\FooClass')->setConfigurator(array($builder->getDefinition('baz'), 'configure'));
420+
$builder->register('foo5', 'Bar\FooClass')->setConfigurator('foo');
421+
422+
$builder->compile();
423+
424+
$this->assertTrue($builder->get('foo1')->configured, '->createService() calls the configurator');
425+
$this->assertTrue($builder->get('foo2')->configured, '->createService() calls the configurator');
426+
$this->assertTrue($builder->get('foo3')->configured, '->createService() calls the configurator');
356427
$this->assertTrue($builder->get('foo4')->configured, '->createService() calls the configurator');
357428

358-
$builder->register('foo5', 'Bar\FooClass')->setConfigurator('foo');
359429
try {
360430
$builder->get('foo5');
361431
$this->fail('->createService() throws an InvalidArgumentException if the configure callable is not a valid callable');
@@ -371,6 +441,9 @@ public function testCreateSyntheticService()
371441
{
372442
$builder = new ContainerBuilder();
373443
$builder->register('foo', 'Bar\FooClass')->setSynthetic(true);
444+
445+
$builder->compile();
446+
374447
$builder->get('foo');
375448
}
376449

@@ -380,13 +453,18 @@ public function testCreateServiceWithExpression()
380453
$builder->setParameter('bar', 'bar');
381454
$builder->register('bar', 'BarClass');
382455
$builder->register('foo', 'Bar\FooClass')->addArgument(array('foo' => new Expression('service("bar").foo ~ parameter("bar")')));
456+
457+
$builder->compile();
458+
383459
$this->assertEquals('foobar', $builder->get('foo')->arguments['foo']);
384460
}
385461

386462
public function testResolveServices()
387463
{
388464
$builder = new ContainerBuilder();
389465
$builder->register('foo', 'Bar\FooClass');
466+
$builder->compile();
467+
390468
$this->assertEquals($builder->get('foo'), $builder->resolveServices(new Reference('foo')), '->resolveServices() resolves service references to service instances');
391469
$this->assertEquals(array('foo' => array('foo', $builder->get('foo'))), $builder->resolveServices(array('foo' => array('foo', new Reference('foo')))), '->resolveServices() resolves service references to service instances in nested arrays');
392470
$this->assertEquals($builder->get('foo'), $builder->resolveServices(new Expression('service("foo")')), '->resolveServices() resolves expressions');

0 commit comments

Comments
 (0)