From 100048da33c7ea2561de008033deed99a59e7723 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Fri, 19 May 2017 13:45:07 +1200 Subject: [PATCH] API PSR-11 compliance (fixes #6594) (#6931) Note that our usage of `$asSingleton` in `get()` is fine. Quote from the PSR: > Two successive calls to get with the same identifier SHOULD return the same value. However, depending on the implementor design and/or user configuration, different values might be returned, so user SHOULD NOT rely on getting the same value on 2 successive calls. --- composer.json | 6 +- docs/en/04_Changelogs/4.0.0.md | 3 + src/Core/Injector/InjectionCreator.php | 7 +- src/Core/Injector/Injector.php | 102 +++++++++++++----- .../Injector/InjectorNotFoundException.php | 9 ++ tests/php/Core/Injector/InjectorTest.php | 40 +++++-- tests/php/ORM/DataListTest.php | 5 +- 7 files changed, 130 insertions(+), 42 deletions(-) create mode 100644 src/Core/Injector/InjectorNotFoundException.php diff --git a/composer.json b/composer.json index f51c805ea..d18988ba6 100644 --- a/composer.json +++ b/composer.json @@ -39,7 +39,8 @@ "ext-tokenizer": "*", "ext-ctype": "*", "ext-hash": "*", - "ext-session": "*" + "ext-session": "*", + "psr/container-implementation": "1.0.0" }, "require-dev": { "phpunit/phpunit": "^5.7", @@ -48,6 +49,9 @@ "silverstripe/serve": "dev-master", "se/selenium-server-standalone": "2.41.0" }, + "provide": { + "psr/container-implementation": "1.0.0" + }, "extra": { "branch-alias": { "dev-master": "4.0.x-dev" diff --git a/docs/en/04_Changelogs/4.0.0.md b/docs/en/04_Changelogs/4.0.0.md index 7f30d97d4..506df190d 100644 --- a/docs/en/04_Changelogs/4.0.0.md +++ b/docs/en/04_Changelogs/4.0.0.md @@ -1292,6 +1292,9 @@ After (`mysite/_config/config.yml`): * Removed `CreditCardField`, `CountryDropdownField`, `PhoneNumberField`, `MemberDatetimeOptionsetField`, `InlineFormAction`. Use custom code instead * Removed `ResetFormAction`, use `FormAction::create()->setAttribute('type', 'reset')` instead +* `Injector` now complies with [PSR-11](https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-11-container.md). + Accordingly, `hasService()` has been renamed to `has()`, and `get()` will throw + `SilverStripe\Core\Injector\InjectorNotFoundException` when the service can't be found. #### General and Core Deprecated API diff --git a/src/Core/Injector/InjectionCreator.php b/src/Core/Injector/InjectionCreator.php index 420e3264f..9f5cbd83e 100644 --- a/src/Core/Injector/InjectionCreator.php +++ b/src/Core/Injector/InjectionCreator.php @@ -3,6 +3,7 @@ namespace SilverStripe\Core\Injector; use ReflectionClass; +use ReflectionException; /** * A class for creating new objects by the injector. @@ -12,7 +13,11 @@ class InjectionCreator implements Factory public function create($class, array $params = array()) { - $reflector = new ReflectionClass($class); + try { + $reflector = new ReflectionClass($class); + } catch (ReflectionException $e) { + throw new InjectorNotFoundException($e); + } if (count($params)) { return $reflector->newInstanceArgs($params); diff --git a/src/Core/Injector/Injector.php b/src/Core/Injector/Injector.php index 62f8be348..235fa258a 100644 --- a/src/Core/Injector/Injector.php +++ b/src/Core/Injector/Injector.php @@ -2,11 +2,14 @@ namespace SilverStripe\Core\Injector; +use Psr\Container\NotFoundExceptionInterface; use SilverStripe\Core\Config\Config; use ReflectionProperty; use ArrayObject; use ReflectionObject; use ReflectionMethod; +use Psr\Container\ContainerInterface; +use SilverStripe\Dev\Deprecation; /** * A simple injection manager that manages creating objects and injecting @@ -122,7 +125,7 @@ use ReflectionMethod; * @author marcus@silverstripe.com.au * @license BSD License http://silverstripe.org/bsd-license/ */ -class Injector +class Injector implements ContainerInterface { /** @@ -233,7 +236,7 @@ class Injector * If a user wants to use the injector as a static reference * * @param array $config - * @return Injector + * @return ContainerInterface */ public static function inst($config = null) { @@ -246,10 +249,10 @@ class Injector /** * Sets the default global injector instance. * - * @param Injector $instance + * @param ContainerInterface $instance * @return Injector Reference to new active Injector instance */ - public static function set_inst(Injector $instance) + public static function set_inst(ContainerInterface $instance) { return self::$instance = $instance; } @@ -693,7 +696,7 @@ class Injector if ($propertyObject->isPublic() && !$propertyObject->getValue($object)) { $origName = $propertyObject->getName(); $name = ucfirst($origName); - if ($this->hasService($name)) { + if ($this->has($name)) { // Pull the name out of the registry $value = $this->get($name); $propertyObject->setValue($object, $value); @@ -710,7 +713,7 @@ class Injector $methName = $methodObj->getName(); if (strpos($methName, 'set') === 0) { $pname = substr($methName, 3); - if ($this->hasService($pname)) { + if ($this->has($pname)) { // Pull the name out of the registry $value = $this->get($pname); $methodObj->invoke($object, $value); @@ -782,6 +785,35 @@ class Injector } } + /** + * @deprecated 4.0.0:5.0.0 Use Injector::has() instead + * @param $name + * @return string + */ + public function hasService($name) + { + Deprecation::notice('5.0', 'Use Injector::has() instead'); + + return $this->has($name); + } + + /** + * Does the given service exist? + * + * We do a special check here for services that are using compound names. For example, + * we might want to say that a property should be injected with Log.File or Log.Memory, + * but have only registered a 'Log' service, we'll instead return that. + * + * Will recursively call itself for each depth of dotting. + * + * @param string $name + * @return boolean + */ + public function has($name) + { + return (bool)$this->getServiceName($name); + } + /** * Does the given service exist, and if so, what's the stored name for it? * @@ -789,15 +821,14 @@ class Injector * we might want to say that a property should be injected with Log.File or Log.Memory, * but have only registered a 'Log' service, we'll instead return that. * - * Will recursively call hasService for each depth of dotting + * Will recursively call itself for each depth of dotting. * * @param string $name - * @return string The name of the service (as it might be different from the one passed in) - * The name of the service (as it might be different from the one passed in) + * @return string|null The name of the service (as it might be different from the one passed in) */ - public function hasService($name) + public function getServiceName($name) { - // common case, get it overwith first + // common case, get it over with first if (isset($this->specs[$name])) { return $name; } @@ -808,7 +839,7 @@ class Injector return null; } - return $this->hasService(substr($name, 0, strrpos($name, '.'))); + return $this->getServiceName(substr($name, 0, strrpos($name, '.'))); } /** @@ -859,30 +890,45 @@ class Injector * Next, will check to see if there's any registered configuration for the given type * and will then try and load that * - * Failing all of that, will just return a new instance of the - * specificied object. + * Failing all of that, will just return a new instance of the specified object. * - * @param string $name - * the name of the service to retrieve. If not a registered - * service, then a class of the given name is instantiated - * @param boolean $asSingleton - * Whether to register the created object as a singleton - * if no other configuration is found - * @param array $constructorArgs - * Optional set of arguments to pass as constructor arguments - * if this object is to be created from scratch - * (ie asSingleton = false) - * @return mixed the instance of the specified object + * @throws NotFoundExceptionInterface No entry was found for **this** identifier. + * + * @param string $name The name of the service to retrieve. If not a registered + * service, then a class of the given name is instantiated + * @param boolean $asSingleton Whether to register the created object as a singleton + * if no other configuration is found + * @param array $constructorArgs Optional set of arguments to pass as constructor arguments + * if this object is to be created from scratch (with $asSingleton = false) + * @return mixed Instance of the specified object */ public function get($name, $asSingleton = true, $constructorArgs = null) + { + $object = $this->getNamedService($name, $asSingleton, $constructorArgs); + + if (!$object) { + throw new InjectorNotFoundException("The '{$name}' service could not be found"); + } + + return $object; + } + + /** + * Returns the service, or `null` if it doesnt' exist. See {@link get()} for main usage. + * + * @param string $name + * @param boolean $asSingleton + * @param array $constructorArgs + * @return mixed|null Instance of the specified object (if it exists) + */ + protected function getNamedService($name, $asSingleton = true, $constructorArgs = null) { // reassign the name as it might actually be a compound name - if ($serviceName = $this->hasService($name)) { + if ($serviceName = $this->getServiceName($name)) { // check to see what the type of bean is. If it's a prototype, // we don't want to return the singleton version of it. $spec = $this->specs[$serviceName]; $type = isset($spec['type']) ? $spec['type'] : null; - // if we're explicitly a prototype OR we're not wanting a singleton if (($type && $type == 'prototype') || !$asSingleton) { if ($spec && $constructorArgs) { @@ -903,7 +949,6 @@ class Injector return $this->serviceCache[$serviceName]; } } - $config = $this->configLocator->locateConfigFor($name); if ($config) { $this->load(array($name => $config)); @@ -916,7 +961,6 @@ class Injector return $this->instantiate($spec, $name); } } - // If we've got this far, we're dealing with a case of a user wanting // to create an object based on its name. So, we need to fake its config // if the user wants it managed as a singleton service style object diff --git a/src/Core/Injector/InjectorNotFoundException.php b/src/Core/Injector/InjectorNotFoundException.php new file mode 100644 index 000000000..ff30fb441 --- /dev/null +++ b/src/Core/Injector/InjectorNotFoundException.php @@ -0,0 +1,9 @@ +load($config); + + + $this->assertFalse($injector->has('UnknownService')); + $this->assertNull($injector->getServiceName('UnknownService')); + + $this->assertTrue($injector->has('SampleService')); $this->assertEquals( 'SampleService', - $injector->hasService('SampleService') + $injector->getServiceName('SampleService') ); $myObject = new TestObject(); @@ -110,15 +117,17 @@ class InjectorTest extends SapphireTest ); $injector->load($services); + $this->assertTrue($injector->has('SampleService')); $this->assertEquals( 'SampleService', - $injector->hasService('SampleService') + $injector->getServiceName('SampleService') ); // We expect a false because the AnotherService::class is actually // just a replacement of the SilverStripe\Core\Tests\Injector\AopProxyServiceTest\SampleService + $this->assertTrue($injector->has('SampleService')); $this->assertEquals( 'AnotherService', - $injector->hasService('AnotherService') + $injector->getServiceName('AnotherService') ); $item = $injector->get('AnotherService'); @@ -136,8 +145,11 @@ class InjectorTest extends SapphireTest $injector->load($services); - $this->assertTrue($injector->hasService('FirstId') == 'FirstId'); - $this->assertTrue($injector->hasService('SecondId') == 'SecondId'); + $this->assertTrue($injector->has('FirstId')); + $this->assertEquals($injector->getServiceName('FirstId'), 'FirstId'); + + $this->assertTrue($injector->has('SecondId')); + $this->assertEquals($injector->getServiceName('SecondId'), 'SecondId'); $this->assertTrue($injector->get('FirstId') instanceof AnotherService); $this->assertTrue($injector->get('SecondId') instanceof SampleService); @@ -251,9 +263,10 @@ class InjectorTest extends SapphireTest ); $injector->load($config); + $this->assertTrue($injector->has('SampleService')); $this->assertEquals( 'SampleService', - $injector->hasService('SampleService') + $injector->getServiceName('SampleService') ); // We expect a false because the AnotherService::class is actually // just a replacement of the SilverStripe\Core\Tests\Injector\AopProxyServiceTest\SampleService @@ -419,7 +432,8 @@ class InjectorTest extends SapphireTest ); $injector->load($config); - $this->assertEquals('SampleService', $injector->hasService('SampleService')); + $this->assertTrue($injector->has('SampleService')); + $this->assertEquals('SampleService', $injector->getServiceName('SampleService')); $myObject = new InjectorTest\OtherTestObject(); $injector->inject($myObject); @@ -453,7 +467,8 @@ class InjectorTest extends SapphireTest ); $injector->load($config); - $this->assertEquals('SampleService', $injector->hasService('SampleService')); + $this->assertTrue($injector->has('SampleService')); + $this->assertEquals('SampleService', $injector->getServiceName('SampleService')); $myObject = $injector->get(OtherTestObject::class); $this->assertInstanceOf( @@ -919,7 +934,14 @@ class InjectorTest extends SapphireTest $item = $injector->get('TestService'); } - + /** + * @expectedException \SilverStripe\Core\Injector\InjectorNotFoundException + */ + public function testGetThrowsOnNotFound() + { + $injector = new Injector(); + $injector->get('UnknownService'); + } /** * Test nesting of injector diff --git a/tests/php/ORM/DataListTest.php b/tests/php/ORM/DataListTest.php index fecca2fd9..703454b57 100755 --- a/tests/php/ORM/DataListTest.php +++ b/tests/php/ORM/DataListTest.php @@ -3,6 +3,7 @@ namespace SilverStripe\ORM\Tests; use SilverStripe\Core\Convert; +use SilverStripe\Core\Injector\InjectorNotFoundException; use SilverStripe\ORM\DataList; use SilverStripe\ORM\DB; use SilverStripe\ORM\Filterable; @@ -744,7 +745,7 @@ class DataListTest extends SapphireTest public function testSimpleFilterWithNonExistingComparisator() { $this->setExpectedException( - 'ReflectionException', + InjectorNotFoundException::class, 'Class DataListFilter.Bogus does not exist' ); $list = TeamComment::get(); @@ -755,7 +756,7 @@ class DataListTest extends SapphireTest { // Invalid modifiers are treated as failed filter construction $this->setExpectedException( - 'ReflectionException', + InjectorNotFoundException::class, 'Class DataListFilter.invalidmodifier does not exist' ); $list = TeamComment::get();