Merge pull request #5440 from tractorcow/pulls/4.0/api-explicit-injector-inheritance

API Injector dependencies no longer inherit from parent classes

control/injector/SilverStripeServiceConfigurationLocator.php

@@ -19,53 +19,36 @@ class SilverStripeServiceConfigurationLocator extends ServiceConfigurationLocato
 	protected $configs = array();
 
 	public function locateConfigFor($name) {
-
 		// Check direct or cached result
 		$config = $this->configFor($name);
-		if($config !== null) return $config;
+		if(!$config) {
-
+			return null;
-		// do parent lookup if it's a class
-		if (class_exists($name)) {
-			$parents = array_reverse(array_values(ClassInfo::ancestry($name)));
-			array_shift($parents);
-
-			foreach ($parents as $parent) {
-				// have we already got for this?
-				$config = $this->configFor($parent);
-				if($config !== null) {
-					// Cache this result
-					$this->configs[$name] = $config;
-					return $config;
-				}
-			}
 		}
 
-		// there is no parent config, so we'll record that as false so we don't do the expensive
+		// If config is in `%$Source` format then inherit from the named config
-		// lookup through parents again
+		if(is_string($config) && stripos($config, '%$') === 0) {
-		$this->configs[$name] = false;
+			$name = substr($config, 2);
+			return $this->locateConfigFor($name);
+		}
+
+		// Return the located config
+		return $config;
 	}
 
 	/**
 	 * Retrieves the config for a named service without performing a hierarchy walk
 	 *
 	 * @param string $name Name of service
-	 * @return mixed Returns either the configuration data, if there is any. A missing config is denoted
+	 * @return mixed Get config for this service
-	 * by a value of either null (there is no direct config assigned and a hierarchy walk is necessary)
-	 * or false (there is no config for this class, nor within the hierarchy for this class).
 	 */
 	protected function configFor($name) {
-
 		// Return cached result
-		if (isset($this->configs[$name])) {
+		if (array_key_exists($name, $this->configs)) {
-			return $this->configs[$name]; // Potentially false
+			return $this->configs[$name];
 		}
 
 		$config = Config::inst()->get('Injector', $name);
-		if ($config) {
+		$this->configs[$name] = $config;
-			$this->configs[$name] = $config;
+		return $config;
-			return $config;
-		} else {
-			return null;
-		}
 	}
 }

docs/en/02_Developer_Guides/05_Extending/05_Injector.md

@@ -225,6 +225,22 @@ Would setup the following
 * Look at the properties to be injected and look for the config for `MySQLDatabase`
 * Create a MySQLDatabase class, passing dbusername and dbpassword as the parameters to the constructor.
 
+## Service inheritance
+
+By default, services registered with Injector do not inherit from one another; This is because it registers
+named services, which may not be actual classes, and thus should not behave as though they were.
+
+Thus if you want an object to have the injected dependencies of a service of another name, you must
+assign a reference to that service.
+
+
+	:::yaml
+	Injector:
+	  JSONServiceDefinition:
+	    properties:
+	      Serialiser: JSONSerialiser
+	  GZIPJSONProvider: %$JSONServiceDefinition
+
 
 ## Testing with Injector
 

docs/en/04_Changelogs/4.0.0.md

@@ -42,6 +42,7 @@
    * `CMSMain.enabled_legacy_actions` config is removed.
  * `DataObject::can` has new method signature with `$context` parameter.
  * `SiteTree.alternatePreviewLink` is deprecated. Use `updatePreviewLink` instead.
+ * `Injector` dependencies no longer automatically inherit from parent classes.
 
 ## New API
 

tests/injector/InjectorTest.php

@@ -487,38 +487,30 @@ class InjectorTest extends SapphireTest {
 
 	public function testInheritedConfig() {
 
-		// Test top-down caching of config inheritance
+		// Test that child class does not automatically inherit config
 		$injector = new Injector(array('locator' => 'SilverStripeServiceConfigurationLocator'));
-		Config::inst()->update('Injector', 'MyParentClass', array('properties' => array('one' => 'the one')));
+		Config::inst()->update('Injector', 'MyParentClass', [
-		Config::inst()->update('Injector', 'MyChildClass', array('properties' => array('one' => 'the two')));
+			'properties' => ['one' => 'the one'],
+			'class' => 'MyParentClass',
+		]);
 		$obj = $injector->get('MyParentClass');
+		$this->assertInstanceOf('MyParentClass', $obj);
 		$this->assertEquals($obj->one, 'the one');
 
+		// Class isn't inherited and parent properties are ignored
 		$obj = $injector->get('MyChildClass');
-		$this->assertEquals($obj->one, 'the two');
+		$this->assertInstanceOf('MyChildClass', $obj);
+		$this->assertNotEquals($obj->one, 'the one');
 
-		$obj = $injector->get('MyGrandChildClass');
+		// Set child class as alias
-		$this->assertEquals($obj->one, 'the two');
-
-		$obj = $injector->get('MyGreatGrandChildClass');
-		$this->assertEquals($obj->one, 'the two');
-
-		// Test bottom-up caching of config inheritance
 		$injector = new Injector(array('locator' => 'SilverStripeServiceConfigurationLocator'));
-		Config::inst()->update('Injector', 'MyParentClass', array('properties' => array('one' => 'the three')));
+		Config::inst()->update('Injector', 'MyChildClass', '%$MyParentClass');
-		Config::inst()->update('Injector', 'MyChildClass', array('properties' => array('one' => 'the four')));
-
-		$obj = $injector->get('MyGreatGrandChildClass');
-		$this->assertEquals($obj->one, 'the four');
-
-		$obj = $injector->get('MyGrandChildClass');
-		$this->assertEquals($obj->one, 'the four');
 
+		// Class isn't inherited and parent properties are ignored
 		$obj = $injector->get('MyChildClass');
-		$this->assertEquals($obj->one, 'the four');
+		$this->assertInstanceOf('MyParentClass', $obj);
+		$this->assertEquals($obj->one, 'the one');
 
-		$obj = $injector->get('MyParentClass');
-		$this->assertEquals($obj->one, 'the three');
 	}
 
 	public function testSameNamedSingeltonPrototype() {
@@ -794,6 +786,7 @@ class ConstructableObject implements TestOnly {
 	}
 }
 
+
 class TestObject implements TestOnly {
 
 	public $sampleService;