From efb004b72aab74c65871c3dbd268a8c453a353d7 Mon Sep 17 00:00:00 2001
From: Damian Mooyman <damian@silverstripe.com>
Date: Mon, 5 Sep 2016 18:18:52 +1200
Subject: [PATCH] API use injector for DataObject::newClassInstance() API Fail
 on invalid class type change

---
 ORM/DataObject.php             | 24 ++++++++++++++----------
 tests/model/DataObjectTest.php |  7 +++++++
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/ORM/DataObject.php b/ORM/DataObject.php
index 9e4f1a44a..e28f43d7f 100644
--- a/ORM/DataObject.php
+++ b/ORM/DataObject.php
@@ -553,14 +553,17 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 	 * or destroy and reinstanciate the record.
 	 *
 	 * @param string $className The new ClassName attribute (a subclass of {@link DataObject})
-	 * @return DataObject $this
+	 * @return $this
 	 */
 	public function setClassName($className) {
 		$className = trim($className);
-		if(!$className || !is_subclass_of($className, 'SilverStripe\ORM\DataObject')) return;
+		if(!$className || !is_subclass_of($className, 'SilverStripe\ORM\DataObject')) {
+			return $this;
+		}
 
 		$this->class = $className;
 		$this->setField("ClassName", $className);
+		$this->setField('RecordClassName', $className);
 		return $this;
 	}
 
@@ -581,15 +584,16 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 	 * @return DataObject The new instance of the new class, The exact type will be of the class name provided.
 	 */
 	public function newClassInstance($newClassName) {
-		$originalClass = $this->ClassName;
-		$newInstance = new $newClassName(array_merge(
-			$this->record,
-			array(
-				'ClassName' => $originalClass,
-				'RecordClassName' => $originalClass,
-			)
-		), false, $this->model);
+		if (!is_subclass_of($newClassName, __CLASS__)) {
+			throw new InvalidArgumentException("$newClassName is not a valid subclass of DataObject");
+		}
 
+		$originalClass = $this->ClassName;
+
+		/** @var DataObject $newInstance */
+		$newInstance = Injector::inst()->create($newClassName, $this->record, false, $this->model);
+
+		// Modify ClassName
 		if($newClassName != $originalClass) {
 			$newInstance->setClassName($newClassName);
 			$newInstance->populateDefaults();
diff --git a/tests/model/DataObjectTest.php b/tests/model/DataObjectTest.php
index 660933003..ebe6db6bc 100644
--- a/tests/model/DataObjectTest.php
+++ b/tests/model/DataObjectTest.php
@@ -1264,14 +1264,21 @@ class DataObjectTest extends SapphireTest {
 		// Don't write the record, it will reset changed fields
 		$this->assertInstanceOf('DataObjectTest_SubTeam', $changedDO);
 		$this->assertEquals($changedDO->ClassName, 'DataObjectTest_SubTeam');
+		$this->assertEquals($changedDO->RecordClassName, 'DataObjectTest_SubTeam');
 		$this->assertContains('ClassName', array_keys($changedFields));
 		$this->assertEquals($changedFields['ClassName']['before'], 'DataObjectTest_Team');
 		$this->assertEquals($changedFields['ClassName']['after'], 'DataObjectTest_SubTeam');
+		$this->assertEquals($changedFields['RecordClassName']['before'], 'DataObjectTest_Team');
+		$this->assertEquals($changedFields['RecordClassName']['after'], 'DataObjectTest_SubTeam');
 
 		$changedDO->write();
 
 		$this->assertInstanceOf('DataObjectTest_SubTeam', $changedDO);
 		$this->assertEquals($changedDO->ClassName, 'DataObjectTest_SubTeam');
+
+		// Test invalid classes fail
+		$this->setExpectedException('InvalidArgumentException', "Controller is not a valid subclass of DataObject");
+		$dataObject->newClassInstance('Controller');
 	}
 
 	public function testMultipleManyManyWithSameClass() {