Merge remote-tracking branch 'origin/3.0' into 3.1

Conflicts:
	admin/javascript/LeftAndMain.AddForm.js
	control/Director.php
	control/HTTPResponse.php
	dev/Profiler.php
	email/Mailer.php
	forms/ComplexTableField.php
	forms/ManyManyComplexTableField.php
	forms/SimpleImageField.php
	forms/TableField.php
	forms/TableListField.php
	javascript/ComplexTableField.js
	javascript/ImageFormAction.js
	javascript/TableField.js
	javascript/TableListField.js
	security/Member.php
	tests/behat/features/bootstrap/SilverStripe/Framework/Test/Behaviour/CmsUiContext.php
	tests/forms/TableListFieldTest.php

control/injector/Injector.php

@@ -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);
 			}
 		}
@@ -404,6 +405,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
 	 * resolved to actual objects
@@ -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
@@ -735,10 +750,17 @@ class Injector {
 			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);
 	}
 }
 

email/Mailer.php

@@ -53,7 +53,7 @@ class Mailer {
 				} else {
 					$messageParts[] = $this->encodeFileForEmail($file);
 	}
-			}
+}
 
 
 			// We further wrap all of this into another multipart block
@@ -233,12 +233,12 @@ class Mailer {
 	}
 	
 	return $result;
-	}
+}
 
 	/**
 	 * @todo Make visibility protected in 3.2
  */
-	function encodeMultipart($parts, $contentType, $headers = false) {
+function encodeMultipart($parts, $contentType, $headers = false) {
 	$separator = "----=_NextPart_" . preg_replace('/[^0-9]/', '', rand() * 10000000000);
 
 	$headers["MIME-Version"] = "1.0";
@@ -261,20 +261,20 @@ class Mailer {
 		$separator . implode("\n".$separator, $parts) . "\n" . trim($separator) . "--";
 
 	return array($body, $headers);
-	}
+}
 
 	/**
 	 * @todo Make visibility protected in 3.2
  */
-	function processHeaders($headers, $body = false) {
+function processHeaders($headers, $body = false) {
 	$res = '';
 	if(is_array($headers)) while(list($k, $v) = each($headers))
 		$res .= "$k: $v\n";
 	if($body) $res .= "\n$body";
 	return $res;
-	}
+}
 
-	/**
+/**
  * Encode the contents of a file for emailing, including headers
  * 
  * $file can be an array, in which case it expects these members:
@@ -316,7 +316,7 @@ class Mailer {
  * 
 	 * @todo Make visibility protected in 3.2
  */
-	function encodeFileForEmail($file, $destFileName = false, $disposition = NULL, $extraHeaders = "") {
+function encodeFileForEmail($file, $destFileName = false, $disposition = NULL, $extraHeaders = "") {
 	if(!$file) {
 		user_error("encodeFileForEmail: not passed a filename and/or data", E_USER_WARNING);
 		return;
@@ -352,7 +352,7 @@ class Mailer {
 
 	$headers =	"Content-type: $mimeType;\n\tname=\"$base\"\n".
 				"Content-Transfer-Encoding: $encoding\n".
-					"Content-Disposition: $disposition;\n\tfilename=\"$base\"\n" ;
+				"Content-Disposition: $disposition;\n\tfilename=\"$base\"\n";
 	
 	if ( isset($file['contentLocation']) ) $headers .= 'Content-Location: ' . $file['contentLocation'] . "\n" ;
 	
@@ -360,12 +360,12 @@ class Mailer {
 
 	// Return completed packet
 	return $headers . $file['contents'];
-	}
+}
 
 	/**
 	 * @todo Make visibility protected in 3.2
 	 */
-	function QuotedPrintable_encode($quotprint) {
+function QuotedPrintable_encode($quotprint) {
 		$quotprint = (string)str_replace('\r\n',chr(13).chr(10),$quotprint);
 		$quotprint = (string)str_replace('\n',  chr(13).chr(10),$quotprint);
 		$quotprint = (string)preg_replace("~([\x01-\x1F\x3D\x7F-\xFF])~e", "sprintf('=%02X', ord('\\1'))", $quotprint);
@@ -375,12 +375,12 @@ class Mailer {
 		$quotprint = (string)str_replace('=0D',"\n",$quotprint);	
 		$quotprint = (string)str_replace('=0A',"\n",$quotprint);	
 		return (string) $quotprint;
-	}
+}
 
 	/**
 	 * @todo Make visibility protected in 3.2
 	 */
-	function validEmailAddr($emailAddress) {
+function validEmailAddr($emailAddress) {
 	$emailAddress = trim($emailAddress);
 	$angBrack = strpos($emailAddress, '<');
 	
@@ -393,7 +393,7 @@ class Mailer {
 	}
 	
 	return $emailAddress;
-	}
+}
 }
 
 /**

tests/injector/InjectorTest.php

@@ -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 create(Injector $injector, $class, $params = array()) {
+	public function __construct($injector) {
+		$this->injector = $injector;
+	}
+
+	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);
 		}
 	}