tonyair/silverstripe-framework
1
0
Fork 0
mirror of https://github.com/silverstripe/silverstripe-framework synced 2024-10-22 14:05:37 +02:00
Code Issues Projects Releases Wiki Activity

BUG Fixed issue with convertServiceProperty

Browse Source 
Fixed issue where convertServiceProperty is called when creating objects
with user-supplied constructor arguments, so that it's only called when
creating objects using injector configuration. This reduces the overhead
of unnecessary calls to convertServiceProperty.

Updated test cases to validate behaviour
This commit is contained in:
Marcus Nyeholt 2012-12-07 13:16:33 +11:00
parent 1e0b0e7f56
commit 4f63f91cc8
2 changed files with 106 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
control/injector/Injector.php
View File

@ -362,6 +362,7 @@ class Injector {
// EXCEPT when there's already an existing instance at this id.
// if so, we need to instantiate and replace immediately
if (isset($this->serviceCache[$id])) {
$this->updateSpecConstructor($spec);
$this->instantiate($spec, $id);
}
}
@ -403,6 +404,20 @@ class Injector {
}
}
}
/**
* Update a class specification to convert constructor configuration information if needed
*
* We do this as a separate process to avoid unneeded calls to convertServiceProperty
*
* @param array $spec
* The class specification to update
*/
protected function updateSpecConstructor(&$spec) {
if (isset($spec['constructor'])) {
$spec['constructor'] = $this->convertServiceProperty($spec['constructor']);
}
}
/**
* Recursively convert a value into its proper representation with service references
@ -468,7 +483,7 @@ class Injector {
$constructorParams = $spec['constructor'];
}
$object = $this->objectCreator->create($this, $class, $constructorParams);
$object = $this->objectCreator->create($class, $constructorParams);
// figure out if we have a specific id set or not. In some cases, we might be instantiating objects
// that we don't manage directly; we don't want to store these in the service cache below
@ -730,15 +745,22 @@ class Injector {
// 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) {
$spec['constructor'] = $constructorArgs;
} else {
// convert any _configured_ constructor args.
// we don't call this for get() calls where someone passes in
// constructor args, otherwise we end up calling convertServiceParams
// way too often
$this->updateSpecConstructor($spec);
}
return $this->instantiate($spec, $serviceName, !$type ? 'prototype' : $type);
} else {
if (!isset($this->serviceCache[$serviceName])) {
$this->updateSpecConstructor($spec);
$this->instantiate($spec, $serviceName);
}
return $this->serviceCache[$serviceName];
@ -750,6 +772,7 @@ class Injector {
$this->load(array($name => $config));
if (isset($this->specs[$name])) {
$spec = $this->specs[$name];
$this->updateSpecConstructor($spec);
return $this->instantiate($spec, $name);
}
}
@ -816,10 +839,10 @@ class InjectionCreator {
* @param array $params
* An array of parameters to be passed to the constructor
*/
public function create(Injector $injector, $class, $params = array()) {
public function create($class, $params = array()) {
$reflector = new ReflectionClass($class);
if (count($params)) {
return $reflector->newInstanceArgs($injector->convertServiceProperty($params));
return $reflector->newInstanceArgs($params);
}
return $reflector->newInstance();
}
@ -833,10 +856,10 @@ class SilverStripeInjectionCreator {
* @param array $params
* An array of parameters to be passed to the constructor
*/
public function create(Injector $injector, $class, $params = array()) {
public function create($class, $params = array()) {
$class = Object::getCustomClass($class);
$reflector = new ReflectionClass($class);
return $reflector->newInstanceArgs($injector->convertServiceProperty($params));
return $reflector->newInstanceArgs($params);
}
}

92
tests/injector/InjectorTest.php
View File

@ -442,7 +442,7 @@ class InjectorTest extends SapphireTest {
public function testCustomObjectCreator() {
$injector = new Injector();
$injector->setObjectCreator(new SSObjectCreator());
$injector->setObjectCreator(new SSObjectCreator($injector));
$config = array(
'OriginalRequirementsBackend',
'DummyRequirements' => array(
@ -485,9 +485,65 @@ class InjectorTest extends SapphireTest {
$again = $injector->get('NeedsBothCirculars');
$this->assertEquals($again->var, 'One');
}
public function testConvertServicePropertyOnCreate() {
// make sure convert service property is not called on direct calls to create, only on configured
// declarations to avoid un-needed function calls
$injector = new Injector();
$item = $injector->create('ConstructableObject', '%$TestObject');
$this->assertEquals('%$TestObject', $item->property);
// do it again but have test object configured as a constructor dependency
$injector = new Injector();
$config = array(
'ConstructableObject' => array(
'constructor' => array(
'%$TestObject'
)
)
);
$injector->load($config);
$item = $injector->get('ConstructableObject');
$this->assertTrue($item->property instanceof TestObject);
// and with a configured object defining TestObject to be something else!
$injector = new Injector(array('locator' => 'InjectorTestConfigLocator'));
$config = array(
'ConstructableObject' => array(
'constructor' => array(
'%$TestObject'
)
),
);
$injector->load($config);
$item = $injector->get('ConstructableObject');
$this->assertTrue($item->property instanceof ConstructableObject);
$this->assertInstanceOf('OtherTestObject', $item->property->property);
}
}
class TestObject {
class InjectorTestConfigLocator extends SilverStripeServiceConfigurationLocator implements TestOnly {
public function locateConfigFor($name) {
if ($name == 'TestObject') {
return array('class' => 'ConstructableObject', 'constructor' => array('%$OtherTestObject'));
}
return parent::locateConfigFor($name);
}
}
class ConstructableObject implements TestOnly {
public $property;
public function __construct($prop) {
$this->property = $prop;
}
}
class TestObject implements TestOnly {
public $sampleService;
@ -497,7 +553,7 @@ class TestObject {
}
class OtherTestObject {
class OtherTestObject implements TestOnly {
private $sampleService;
@ -511,13 +567,13 @@ class OtherTestObject {
}
class CircularOne {
class CircularOne implements TestOnly {
public $circularTwo;
}
class CircularTwo {
class CircularTwo implements TestOnly {
public $circularOne;
@ -528,7 +584,7 @@ class CircularTwo {
}
}
class NeedsBothCirculars {
class NeedsBothCirculars implements TestOnly{
public $circularOne;
public $circularTwo;
@ -536,15 +592,15 @@ class NeedsBothCirculars {
}
class MyParentClass {
class MyParentClass implements TestOnly {
public $one;
}
class MyChildClass extends MyParentClass {
class MyChildClass extends MyParentClass implements TestOnly {
}
class DummyRequirements {
class DummyRequirements implements TestOnly {
public $backend;
@ -558,15 +614,15 @@ class DummyRequirements {
}
class OriginalRequirementsBackend {
class OriginalRequirementsBackend implements TestOnly {
}
class NewRequirementsBackend {
class NewRequirementsBackend implements TestOnly {
}
class TestStaticInjections {
class TestStaticInjections implements TestOnly {
public $backend;
static $dependencies = array(
@ -582,13 +638,19 @@ class TestStaticInjections {
* @see https://github.com/silverstripe/sapphire
*/
class SSObjectCreator extends InjectionCreator {
private $injector;
public function __construct($injector) {
$this->injector = $injector;
}
public function create(Injector $injector, $class, $params = array()) {
public function create($class, $params = array()) {
if (strpos($class, '(') === false) {
return parent::create($injector, $class, $params);
return parent::create($class, $params);
} else {
list($class, $params) = self::parse_class_spec($class);
return parent::create($injector, $class, $params);
$params = $this->injector->convertServiceProperty($params);
return parent::create($class, $params);
}
}