From 70179c8b873676cf2408afc1e5af9fb9e8e9e6c2 Mon Sep 17 00:00:00 2001
From: Damian Mooyman <damian@silverstripe.com>
Date: Mon, 25 Jul 2016 10:05:03 +1200
Subject: [PATCH] API Better behaviour for virtualised controller

---
 code/model/VirtualPage.php      | 79 +++++++++++++++++++++++----------
 tests/model/VirtualPageTest.php | 21 +++++++++
 tests/model/VirtualPageTest.yml |  7 +++
 3 files changed, 83 insertions(+), 24 deletions(-)

diff --git a/code/model/VirtualPage.php b/code/model/VirtualPage.php
index 7105faf7..5694f712 100644
--- a/code/model/VirtualPage.php
+++ b/code/model/VirtualPage.php
@@ -2,6 +2,7 @@
 
 use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\Versioning\Versioned;
+use SilverStripe\Security\Member;
 
 /**
 * Virtual Page creates an instance of a  page, with the same fields that the original page had, but readonly.
@@ -9,6 +10,7 @@ use SilverStripe\ORM\Versioning\Versioned;
 * Note: This Only duplicates $db fields and not the $has_one etc..
  *
  * @method SiteTree CopyContentFrom()
+ * @property int $CopyContentFromID
  *
 * @package cms
 */
@@ -317,7 +319,7 @@ class VirtualPage extends Page {
 		unset($this->components['CopyContentFrom']);
 
 		// Update ImageTracking
-		$this->ImageTracking()->setByIdList($this->CopyContentFrom()->ImageTracking()->column('ID'));
+		$this->ImageTracking()->setByIDList($this->CopyContentFrom()->ImageTracking()->column('ID'));
 	}
 
 	/**
@@ -338,11 +340,14 @@ class VirtualPage extends Page {
 	public function __get($field) {
 		if(parent::hasMethod($funcName = "get$field")) {
 			return $this->$funcName();
-		} else if(parent::hasField($field) || ($field === 'ID' && !$this->exists())) {
+		}
+		if(parent::hasField($field) || ($field === 'ID' && !$this->exists())) {
 			return $this->getField($field);
-		} elseif(($copy = $this->CopyContentFrom()) && $copy->exists()) {
+		}
+		if(($copy = $this->CopyContentFrom()) && $copy->exists()) {
 			return $copy->$field;
 		}
+		return null;
 	}
 
 	public function getField($field) {
@@ -386,7 +391,7 @@ class VirtualPage extends Page {
 		if(parent::hasMethod($method)) {
 			return parent::__call($method, $args);
 		} else {
-			return call_user_func_array(array($this->copyContentFrom(), $method), $args);
+			return call_user_func_array(array($this->CopyContentFrom(), $method), $args);
 		}
 	}
 
@@ -447,11 +452,38 @@ class VirtualPage_Controller extends Page_Controller {
 		'loadcontentall' => 'ADMIN',
 	);
 
+	/**
+	 * Backup of virtualised controller
+	 *
+	 * @var ContentController
+	 */
+	protected $virtualController = null;
+
+	/**
+	 * Get virtual controller
+	 *
+	 * @return ContentController
+	 */
+	protected function getVirtualisedController() {
+		if($this->virtualController) {
+			return $this->virtualController;
+		}
+
+		// Validate virtualised model
+		/** @var VirtualPage $page */
+		$page = $this->data();
+		$virtualisedPage = $page->CopyContentFrom();
+		if (!$virtualisedPage || !$virtualisedPage->exists()) {
+			return null;
+		}
+
+		// Create controller using standard mechanism
+		$this->virtualController = ModelAsController::controller_for($virtualisedPage);
+		return $this->virtualController;
+	}
+
 	public function getViewer($action) {
-		$originalClass = get_class($this->CopyContentFrom());
-		if ($originalClass == 'SiteTree') $name = 'Page_Controller';
-		else $name = $originalClass."_Controller";
-		$controller = new $name();
+		$controller = $this->getVirtualisedController() ?: $this;
 		return $controller->getViewer($action);
 	}
 
@@ -473,15 +505,13 @@ class VirtualPage_Controller extends Page_Controller {
 	 * @return bool
 	 */
 	public function hasMethod($method) {
-		$haveIt = parent::hasMethod($method);
-		if (!$haveIt) {
-			$originalClass = get_class($this->CopyContentFrom());
-			if ($originalClass == 'SiteTree') $name = 'ContentController';
-			else $name = $originalClass."_Controller";
-			$controller = new $name($this->dataRecord->copyContentFrom());
-			$haveIt = $controller->hasMethod($method);
-		}
-		return $haveIt;
+		if(parent::hasMethod($method)) {
+			return true;
+		};
+
+		// Fallback
+		$controller = $this->getVirtualisedController();
+		return $controller && $controller->hasMethod($method);
 	}
 
 	/**
@@ -493,23 +523,24 @@ class VirtualPage_Controller extends Page_Controller {
 	 *
 	 * @throws Exception Any error other than a 'no method' error.
 	 */
-	public function __call($method, $args) {
+	public function __call($method, $args)
+	{
 		// Check if we can safely call this method before passing it back
 		// to custom methods.
-		if($this->getExtraMethodConfig($method)) {
+		if ($this->getExtraMethodConfig($method)) {
 			return parent::__call($method, $args);
 		}
 
 		// Pass back to copied page
-		$original = $this->copyContentFrom();
-		$controller = ModelAsController::controller_for($original);
+		$controller = $this->getVirtualisedController();
+		if(!$controller) {
+			return null;
+		}
 
 		// Ensure request/response data is available on virtual controller
 		$controller->setRequest($this->getRequest());
-		$controller->response = $this->response; // @todo - replace with getter/setter in 3.3
+		$controller->setResponse($this->getResponse());
 
 		return call_user_func_array(array($controller, $method), $args);
 	}
 }
-
-
diff --git a/tests/model/VirtualPageTest.php b/tests/model/VirtualPageTest.php
index 04875846..13caa3a3 100644
--- a/tests/model/VirtualPageTest.php
+++ b/tests/model/VirtualPageTest.php
@@ -601,6 +601,17 @@ class VirtualPageTest extends FunctionalTest {
 		$this->assertEquals(301, $response->getStatusCode());
 		$this->assertEquals('http://google.com', $response->getHeader('Location'));
 	}
+
+	public function testMethod() {
+		$virtualPage = $this->objFromFixture('VirtualPage', 'vp4');
+		$controller = ModelAsController::controller_for($virtualPage);
+
+		$this->assertInstanceOf('VirtualPage_Controller', $controller);
+		$this->assertTrue($controller->hasMethod('testMethod'));
+		$this->assertEquals('hello', $controller->testMethod());
+		$this->assertTrue($controller->hasMethod('modelMethod'));
+		$this->assertEquals('hi there', $controller->modelMethod());
+	}
 }
 
 class VirtualPageTest_ClassA extends Page implements TestOnly {
@@ -613,6 +624,16 @@ class VirtualPageTest_ClassA extends Page implements TestOnly {
 	);
 
 	private static $allowed_children = array('VirtualPageTest_ClassB');
+
+	public function modelMethod() {
+		return 'hi there';
+	}
+}
+
+class VirtualPageTest_ClassA_Controller extends Page_Controller implements TestOnly {
+	public function testMethod() {
+		return 'hello';
+	}
 }
 
 class VirtualPageTest_ClassB extends Page implements TestOnly {
diff --git a/tests/model/VirtualPageTest.yml b/tests/model/VirtualPageTest.yml
index 2bee7257..b5894eac 100644
--- a/tests/model/VirtualPageTest.yml
+++ b/tests/model/VirtualPageTest.yml
@@ -45,6 +45,10 @@ Page:
     CanEditType: OnlyTheseUsers
     CanViewType: Inherit
     EditorGroups: =>SilverStripe\Security\Group.bobgroup
+VirtualPageTest_ClassA:
+  pagea:
+    Title: 'Page A'
+    Content: '<p>Content</p>'
 VirtualPage:
   vp1:
     Title: vp1
@@ -61,3 +65,6 @@ VirtualPage:
     CanViewType: OnlyTheseUsers
     EditorGroups: =>SilverStripe\Security\Group.andrewgroup
     ViewerGroups: =>SilverStripe\Security\Group.cindygroup
+  vp4:
+    CopyContentFrom: =>VirtualPageTest_ClassA.pagea
+    Title: 'vp4'