Skip to content

Commit ee01786

Browse files
fabpotfabpot
committed
bug #17719 [DependencyInjection] fixed exceptions thrown by get method of ContainerBuilder (lukaszmakuch)
This PR was squashed before being merged into the 2.3 branch (closes #17719). Discussion ---------- [DependencyInjection] fixed exceptions thrown by get method of ContainerBuilder [DependencyInjection] fixed exceptions thrown by get method of ContainerBuilder Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | The ContainerBuilder class wasn't implementing the ContainerInterface interface as it should according to the Liskov substitution principle. It caused dependency on implementation instead than on the interface when using an instance of the ContainerBuilder class. For example this code: ```php <?php use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException; use Symfony\Component\DependencyInjection\ContainerInterface; /* @var $container ContainerInterface */ try { $container->get("wrong_service_key"); } catch (ServiceNotFoundException $e) { //action on a wrong key } ``` works for correct implementations of the ContainerInterface interface, but the ContainerBuilder class was breaking that by throwing more abstract exceptions. As the ServiceNotFoundException exceptions inherits from the InvalidArgumentException exception, this change shouldn't break code which catches the InvalidArgumentException exception while fetching values from a ContainerInterface interface implementation. Commits ------- aecb0fa [DependencyInjection] fixed exceptions thrown by get method of ContainerBuilder
2 parents 2ac436c + aecb0fa commit ee01786

File tree

2 files changed

+18
-13
lines changed

2 files changed

+18
-13
lines changed

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

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
2020
use Symfony\Component\DependencyInjection\Exception\LogicException;
2121
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
22+
use Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException;
23+
use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException;
2224
use Symfony\Component\DependencyInjection\Extension\ExtensionInterface;
2325
use Symfony\Component\Config\Resource\FileResource;
2426
use Symfony\Component\Config\Resource\ResourceInterface;
@@ -415,9 +417,9 @@ public function has($id)
415417
*
416418
* @return object The associated service
417419
*
418-
* @throws InvalidArgumentException when no definitions are available
419-
* @throws InactiveScopeException when the current scope is not active
420-
* @throws LogicException when a circular dependency is detected
420+
* @throws InvalidArgumentException when no definitions are available
421+
* @throws ServiceCircularReferenceException When a circular reference is detected
422+
* @throws ServiceNotFoundException When the service is not defined
421423
* @throws \Exception
422424
*
423425
* @see Reference
@@ -440,7 +442,8 @@ public function get($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_INV
440442

441443
try {
442444
$definition = $this->getDefinition($id);
443-
} catch (InvalidArgumentException $e) {
445+
} catch (ServiceNotFoundException $e) {
446+
444447
if (ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE !== $invalidBehavior) {
445448
return;
446449
}
@@ -790,14 +793,14 @@ public function hasDefinition($id)
790793
*
791794
* @return Definition A Definition instance
792795
*
793-
* @throws InvalidArgumentException if the service definition does not exist
796+
* @throws ServiceNotFoundException if the service definition does not exist
794797
*/
795798
public function getDefinition($id)
796799
{
797800
$id = strtolower($id);
798801

799802
if (!array_key_exists($id, $this->definitions)) {
800-
throw new InvalidArgumentException(sprintf('The service definition "%s" does not exist.', $id));
803+
throw new ServiceNotFoundException($id);
801804
}
802805

803806
return $this->definitions[$id];
@@ -812,7 +815,7 @@ public function getDefinition($id)
812815
*
813816
* @return Definition A Definition instance
814817
*
815-
* @throws InvalidArgumentException if the service definition does not exist
818+
* @throws ServiceNotFoundException if the service definition does not exist
816819
*/
817820
public function findDefinition($id)
818821
{

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121
use Symfony\Component\DependencyInjection\Definition;
2222
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
2323
use Symfony\Component\DependencyInjection\Exception\InactiveScopeException;
24+
use Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException;
25+
use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException;
2426
use Symfony\Component\DependencyInjection\Loader\ClosureLoader;
2527
use Symfony\Component\DependencyInjection\Reference;
2628
use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag;
@@ -50,9 +52,9 @@ public function testDefinitions()
5052

5153
try {
5254
$builder->getDefinition('baz');
53-
$this->fail('->getDefinition() throws an InvalidArgumentException if the service definition does not exist');
54-
} catch (\InvalidArgumentException $e) {
55-
$this->assertEquals('The service definition "baz" does not exist.', $e->getMessage(), '->getDefinition() throws an InvalidArgumentException if the service definition does not exist');
55+
$this->fail('->getDefinition() throws a ServiceNotFoundException if the service definition does not exist');
56+
} catch (ServiceNotFoundException $e) {
57+
$this->assertEquals('You have requested a non-existent service "baz".', $e->getMessage(), '->getDefinition() throws a ServiceNotFoundException if the service definition does not exist');
5658
}
5759
}
5860

@@ -79,9 +81,9 @@ public function testGet()
7981
$builder = new ContainerBuilder();
8082
try {
8183
$builder->get('foo');
82-
$this->fail('->get() throws an InvalidArgumentException if the service does not exist');
83-
} catch (\InvalidArgumentException $e) {
84-
$this->assertEquals('The service definition "foo" does not exist.', $e->getMessage(), '->get() throws an InvalidArgumentException if the service does not exist');
84+
$this->fail('->get() throws a ServiceNotFoundException if the service does not exist');
85+
} catch (ServiceNotFoundException $e) {
86+
$this->assertEquals('You have requested a non-existent service "foo".', $e->getMessage(), '->get() throws a ServiceNotFoundException if the service does not exist');
8587
}
8688

8789
$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');

0 commit comments

Comments
 (0)