Merge pull request #4500 from stevie-mayhew/pulls/get-response

FEATURE: implement getter and setter usage for response

admin/code/AdminRootController.php

@@ -77,9 +77,8 @@ class AdminRootController extends Controller {
 			$base = $this->config()->url_base;
 			$segment = Config::inst()->get($this->config()->default_panel, 'url_segment');
 
-			$this->response = new SS_HTTPResponse();
 			$this->redirect(Controller::join_links($base, $segment));
-			return $this->response;
+			return $this->getResponse();
 		}
 
 		// Otherwise

admin/code/LeftAndMain.php

@@ -227,7 +227,7 @@ class LeftAndMain extends Controller implements PermissionProvider {
 
 		// Allow customisation of the access check by a extension
 		// Also all the canView() check to execute Controller::redirect()
-		if(!$this->canView() && !$this->response->isFinished()) {
+		if(!$this->canView() && !$this->getResponse()->isFinished()) {
 			// When access /admin/, we should try a redirect to another part of the admin rather than be locked out
 			$menu = $this->MainMenu();
 			foreach($menu as $candidate) {
@@ -451,8 +451,10 @@ class LeftAndMain extends Controller implements PermissionProvider {
 			$msgs = _t('LeftAndMain.ValidationError', 'Validation error') . ': '
 				. $e->getMessage();
 			$e = new SS_HTTPResponse_Exception($msgs, 403);
-			$e->getResponse()->addHeader('Content-Type', 'text/plain');
+			$errorResponse = $e->getResponse();
-			$e->getResponse()->addHeader('X-Status', rawurlencode($msgs));
+			$errorResponse->addHeader('Content-Type', 'text/plain');
+			$errorResponse->addHeader('X-Status', rawurlencode($msgs));
+			$e->setResponse($errorResponse);
 			throw $e;
 		}
 
@@ -461,8 +463,9 @@ class LeftAndMain extends Controller implements PermissionProvider {
 		if(!$response->getHeader('X-Title')) $response->addHeader('X-Title', urlencode($title));
 
 		// Prevent clickjacking, see https://developer.mozilla.org/en-US/docs/HTTP/X-Frame-Options
-		$this->response->addHeader('X-Frame-Options', 'SAMEORIGIN');
+		$originalResponse = $this->getResponse();
-		$this->response->addHeader('Vary', 'X-Requested-With');
+		$originalResponse->addHeader('X-Frame-Options', 'SAMEORIGIN');
+		$originalResponse->addHeader('Vary', 'X-Requested-With');
 
 		return $response;
 	}
@@ -477,21 +480,21 @@ class LeftAndMain extends Controller implements PermissionProvider {
 	 */
 	public function redirect($url, $code=302) {
 		if($this->getRequest()->isAjax()) {
-			$this->response->addHeader('X-ControllerURL', $url);
+			$response = $this->getResponse();
-			if($this->getRequest()->getHeader('X-Pjax') && !$this->response->getHeader('X-Pjax')) {
+			$response->addHeader('X-ControllerURL', $url);
-				$this->response->addHeader('X-Pjax', $this->getRequest()->getHeader('X-Pjax'));
+			if($this->getRequest()->getHeader('X-Pjax') && !$response->getHeader('X-Pjax')) {
+				$response->addHeader('X-Pjax', $this->getRequest()->getHeader('X-Pjax'));
 			}
-			$oldResponse = $this->response;
 			$newResponse = new LeftAndMain_HTTPResponse(
-				$oldResponse->getBody(),
+				$response->getBody(),
-				$oldResponse->getStatusCode(),
+				$response->getStatusCode(),
-				$oldResponse->getStatusDescription()
+				$response->getStatusDescription()
 			);
-			foreach($oldResponse->getHeaders() as $k => $v) {
+			foreach($response->getHeaders() as $k => $v) {
 				$newResponse->addHeader($k, $v);
 			}
 			$newResponse->setIsFinished(true);
-			$this->response = $newResponse;
+			$this->setResponse($newResponse);
 			return ''; // Actual response will be re-requested by client
 		} else {
 			parent::redirect($url, $code);
@@ -596,7 +599,7 @@ class LeftAndMain extends Controller implements PermissionProvider {
 						return $controller->renderWith($controller->getViewer('show'));
 					}
 				),
-				$this->response
+				$this->getResponse()
 			);
 		}
 		return $this->responseNegotiator;
@@ -1004,7 +1007,7 @@ class LeftAndMain extends Controller implements PermissionProvider {
 				'PrevID' => $prev ? $prev->ID : null
 			);
 		}
-		$this->response->addHeader('Content-Type', 'text/json');
+		$this->getResponse()->addHeader('Content-Type', 'text/json');
 		return Convert::raw2json($data);
 	}
 
@@ -1031,7 +1034,7 @@ class LeftAndMain extends Controller implements PermissionProvider {
 		$this->extend('onAfterSave', $record);
 		$this->setCurrentPageID($record->ID);
 
-		$this->response->addHeader('X-Status', rawurlencode(_t('LeftAndMain.SAVEDUP', 'Saved.')));
+		$this->getResponse()->addHeader('X-Status', rawurlencode(_t('LeftAndMain.SAVEDUP', 'Saved.')));
 		return $this->getResponseNegotiator()->respond($this->getRequest());
 	}
 
@@ -1045,7 +1048,7 @@ class LeftAndMain extends Controller implements PermissionProvider {
 
 		$record->delete();
 
-		$this->response->addHeader('X-Status', rawurlencode(_t('LeftAndMain.DELETED', 'Deleted.')));
+		$this->getResponse()->addHeader('X-Status', rawurlencode(_t('LeftAndMain.DELETED', 'Deleted.')));
 		return $this->getResponseNegotiator()->respond(
 			$this->getRequest(),
 			array('currentform' => array($this, 'EmptyForm'))
@@ -1066,7 +1069,7 @@ class LeftAndMain extends Controller implements PermissionProvider {
 	 */
 	public function savetreenode($request) {
 		if (!Permission::check('SITETREE_REORGANISE') && !Permission::check('ADMIN')) {
-			$this->response->setStatusCode(
+			$this->getResponse()->setStatusCode(
 				403,
 				_t('LeftAndMain.CANT_REORGANISE',
 					"You do not have permission to rearange the site tree. Your change was not saved.")
@@ -1082,7 +1085,7 @@ class LeftAndMain extends Controller implements PermissionProvider {
 		if($className == 'SiteTree' && $page = DataObject::get_by_id('Page', $id)){
 			$root = $page->getParentType();
 			if(($parentID == '0' || $root == 'root') && !SiteConfig::current_site_config()->canCreateTopLevel()){
-				$this->response->setStatusCode(
+				$this->getResponse()->setStatusCode(
 					403,
 					_t('LeftAndMain.CANT_REORGANISE',
 						"You do not have permission to alter Top level pages. Your change was not saved.")
@@ -1099,7 +1102,7 @@ class LeftAndMain extends Controller implements PermissionProvider {
 		if($node && !$node->canEdit()) return Security::permissionFailure($this);
 
 		if(!$node) {
-			$this->response->setStatusCode(
+			$this->getResponse()->setStatusCode(
 				500,
 				_t('LeftAndMain.PLEASESAVE',
 					"Please Save Page: This page could not be updated because it hasn't been saved yet."
@@ -1127,7 +1130,7 @@ class LeftAndMain extends Controller implements PermissionProvider {
 				}
 			}
 
-			$this->response->addHeader('X-Status',
+			$this->getResponse()->addHeader('X-Status',
 				rawurlencode(_t('LeftAndMain.REORGANISATIONSUCCESSFUL', 'Reorganised the site tree successfully.')));
 		}
 
@@ -1152,7 +1155,7 @@ class LeftAndMain extends Controller implements PermissionProvider {
 				}
 			}
 
-			$this->response->addHeader('X-Status',
+			$this->getResponse()->addHeader('X-Status',
 				rawurlencode(_t('LeftAndMain.REORGANISATIONSUCCESSFUL', 'Reorganised the site tree successfully.')));
 		}
 

control/Controller.php

@@ -133,7 +133,6 @@ class Controller extends RequestHandler implements TemplateGlobalProvider {
 		$this->pushCurrent();
 		$this->urlParams = $request->allParams();
 		$this->setRequest($request);
-		$this->response = new SS_HTTPResponse();
 		$this->setDataModel($model);
 
 		$this->extend('onBeforeInit');
@@ -148,10 +147,11 @@ class Controller extends RequestHandler implements TemplateGlobalProvider {
 
 		$this->extend('onAfterInit');
 
+		$response = $this->getResponse();
 		// If we had a redirection or something, halt processing.
-		if($this->response->isFinished()) {
+		if($response->isFinished()) {
 			$this->popCurrent();
-			return $this->response;
+			return $response;
 		}
 
 		$body = parent::handleRequest($request, $model);
@@ -160,7 +160,8 @@ class Controller extends RequestHandler implements TemplateGlobalProvider {
 				Debug::message("Request handler returned SS_HTTPResponse object to $this->class controller;"
 					. "returning it without modification.");
 			}
-			$this->response = $body;
+			$response = $body;
+			$this->setResponse($response);
 
 		} else {
 			if($body instanceof Object && $body->hasMethod('getViewer')) {
@@ -171,15 +172,15 @@ class Controller extends RequestHandler implements TemplateGlobalProvider {
 				$body = $body->getViewer($this->getAction())->process($body);
 			}
 
-			$this->response->setBody($body);
+			$response->setBody($body);
 		}
 
 
-		ContentNegotiator::process($this->response);
+		ContentNegotiator::process($response);
-		HTTP::add_cache_headers($this->response);
+		HTTP::add_cache_headers($response);
 
 		$this->popCurrent();
-		return $this->response;
+		return $response;
 	}
 
 	/**
@@ -226,9 +227,23 @@ class Controller extends RequestHandler implements TemplateGlobalProvider {
 	 * Can be used to set the status code and headers
 	 */
 	public function getResponse() {
+		if (!$this->response) {
+			$this->setResponse(new SS_HTTPResponse());
+		}
 		return $this->response;
 	}
 
+	/**
+	 * Sets the SS_HTTPResponse object that this controller is building up.
+	 *
+	 * @param SS_HTTPResponse $response
+	 * @return Controller
+	 */
+	public function setResponse(SS_HTTPResponse $response) {
+		$this->response = $response;
+		return $this;
+	}
+
 	protected $baseInitCalled = false;
 
 	/**
@@ -468,10 +483,9 @@ class Controller extends RequestHandler implements TemplateGlobalProvider {
 	 * @return SS_HTTPResponse
 	 */
 	public function redirect($url, $code=302) {
-		if(!$this->response) $this->response = new SS_HTTPResponse();
 
-		if($this->response->getHeader('Location') && $this->response->getHeader('Location') != $url) {
+		if($this->getResponse()->getHeader('Location') && $this->getResponse()->getHeader('Location') != $url) {
-			user_error("Already directed to " . $this->response->getHeader('Location')
+			user_error("Already directed to " . $this->getResponse()->getHeader('Location')
 				. "; now trying to direct to $url", E_USER_WARNING);
 			return;
 		}
@@ -481,7 +495,7 @@ class Controller extends RequestHandler implements TemplateGlobalProvider {
 			$url = Director::baseURL() . $url;
 		}
 
-		return $this->response->redirect($url, $code);
+		return $this->getResponse()->redirect($url, $code);
 	}
 
 	/**
@@ -529,7 +543,7 @@ class Controller extends RequestHandler implements TemplateGlobalProvider {
 	 * return null;
 	 */
 	public function redirectedTo() {
-		return $this->response && $this->response->getHeader('Location');
+		return $this->getResponse() && $this->getResponse()->getHeader('Location');
 	}
 
 	/**

docs/en/02_Developer_Guides/02_Controllers/01_Introduction.md

@@ -89,20 +89,20 @@ Action methods can return one of four main things:
 	 * We can manually create a response and return that to ignore any previous data.
 	 */
 	public function someaction(SS_HTTPRequest $request) {
-		$this->response = new SS_HTTPResponse();
+		$this->setResponse(new SS_HTTPResponse());
-		$this->response->setStatusCode(400);
+		$this->getResponse()->setStatusCode(400);
-		$this->response->setBody('invalid');
+		$this->getResponse()->setBody('invalid');
 
-		return $this->response;
+		return $this->getResponse();
 	}
 
 	/**
 	 * Or, we can modify the response that is waiting to go out.
 	 */
 	public function anotheraction(SS_HTTPRequest $request) {
-		$this->response->setStatusCode(400);
+		$this->getResponse()->setStatusCode(400);
 
-		return $this->response;
+		return $this->getResponse();
 	}
 
 	/**
@@ -118,13 +118,13 @@ Action methods can return one of four main things:
 	 * We can send stuff to the browser which isn't HTML
 	 */
 	public function ajaxaction() {
-		$this->response->setBody(json_encode(array(
+		$this->getResponse()->setBody(json_encode(array(
 			'json' => true
 		)));
 
-		$this->response->addHeader("Content-type", "application/json");
+		$this->getResponse()->addHeader("Content-type", "application/json");
 
-		return $this->response.
+		return $this->getResponse().
 	}
 
 For more information on how a URL gets mapped to an action see the [Routing](routing) documentation.

docs/en/02_Developer_Guides/02_Controllers/02_Routing.md

@@ -44,7 +44,7 @@ which will be filled when the user makes their request. Request parameters are a
 and able to be pulled out from a controller using `$this->getRequest()->param($name)`.
 
 <div class="info" markdown="1">
-All Controllers have access to `$this->getRequest()` for the request object and `$this->response` for the response.
+All Controllers have access to `$this->getRequest()` for the request object and `$this->getResponse()` for the response.
 </div>
 
 Here is what those parameters would look like for certain requests

docs/en/02_Developer_Guides/09_Security/04_Secure_Coding.md

@@ -543,7 +543,7 @@ controller's `init()` method:
 	class MyController extends Controller {
 		public function init() {
 			parent::init();
-			$this->response->addHeader('X-Frame-Options', 'SAMEORIGIN');
+			$this->getResponse()->addHeader('X-Frame-Options', 'SAMEORIGIN');
 		}
 	}
 	

docs/en/02_Developer_Guides/15_Customising_the_Admin_Interface/05_CMS_Architecture.md

@@ -360,7 +360,7 @@ without affecting the response body.
 	class MyController extends LeftAndMain {
 		class myaction() {
 			// ...
-			$this->response->addHeader('X-Controller', 'MyOtherController');
+			$this->getResponse()->addHeader('X-Controller', 'MyOtherController');
 			return $html;
 		}
 	}

docs/en/02_Developer_Guides/15_Customising_the_Admin_Interface/06_Javascript_Development.md

@@ -376,7 +376,7 @@ PHP:
 	    if(!$results) return new HTTPResponse("Not found", 404);
 
 	    // Use HTTPResponse to pass custom status messages
-	    $this->response->setStatusCode(200, "Found " . $results->Count() . " elements");
+	    $this->getResponse()->setStatusCode(200, "Found " . $results->Count() . " elements");
 
 	    // render all results with a custom template
 	    $vd = new ViewableData();

forms/HtmlEditorField.php

@@ -897,16 +897,17 @@ class HtmlEditorField_Embed extends HtmlEditorField_File {
 		$this->oembed = Oembed::get_oembed_from_url($url);
 		if(!$this->oembed) {
 			$controller = Controller::curr();
-			$controller->response->addHeader('X-Status',
+			$response = $controller->getResponse();
+			$response->addHeader('X-Status',
 				rawurlencode(_t(
 					'HtmlEditorField.URLNOTANOEMBEDRESOURCE',
 					"The URL '{url}' could not be turned into a media resource.",
 					"The given URL is not a valid Oembed resource; the embed element couldn't be created.",
 					array('url' => $url)
 				)));
-			$controller->response->setStatusCode(404);
+			$response->setStatusCode(404);
 
-			throw new SS_HTTPResponse_Exception($controller->response);
+			throw new SS_HTTPResponse_Exception($response);
 		}
 	}
 

security/CMSSecurity.php

@@ -107,8 +107,9 @@ class CMSSecurity extends Security {
 			'Message displayed to user if their session cannot be restored',
 			array('link' => $loginURLATT)
 		);
-		$this->response->setStatusCode(200);
+		$response = $this->getResponse();
-		$this->response->setBody(<<<PHP
+		$response->setStatusCode(200);
+		$response->setBody(<<<PHP
 <!DOCTYPE html>
 <html><body>
 $message
@@ -118,7 +119,8 @@ setTimeout(function(){top.location.href = "$loginURLJS";}, 0);
 </body></html>
 PHP
 		);
-		return $this->response;
+		$this->setResponse($response);
+		return $response;
 	}
 
 	protected function preLogin() {

security/Security.php

@@ -305,7 +305,7 @@ class Security extends Controller implements TemplateGlobalProvider {
 		parent::init();
 
 		// Prevent clickjacking, see https://developer.mozilla.org/en-US/docs/HTTP/X-Frame-Options
-		$this->response->addHeader('X-Frame-Options', 'SAMEORIGIN');
+		$this->getResponse()->addHeader('X-Frame-Options', 'SAMEORIGIN');
 	}
 
 	public function index() {
@@ -391,7 +391,7 @@ class Security extends Controller implements TemplateGlobalProvider {
 		$member = Member::currentUser();
 		if($member) $member->logOut();
 
-		if($redirect && (!$this->response || !$this->response->isFinished())) {
+		if($redirect && (!$this->getResponse()->isFinished())) {
 			$this->redirectBack();
 		}
 	}
@@ -406,7 +406,7 @@ class Security extends Controller implements TemplateGlobalProvider {
 		// Event handler for pre-login, with an option to let it break you out of the login form
 		$eventResults = $this->extend('onBeforeSecurityLogin');
 		// If there was a redirection, return
-		if($this->redirectedTo()) return $this->response;
+		if($this->redirectedTo()) return $this->getResponse();
 		// If there was an SS_HTTPResponse object returned, then return that
 		if($eventResults) {
 			foreach($eventResults as $result) {

tests/FakeController.php

@@ -18,7 +18,7 @@ class FakeController extends Controller {
 		);
 		$this->setRequest($request);
 
-		$this->response = new SS_HTTPResponse();
+		$this->setResponse(new SS_HTTPResponse());
 
 		$this->init();
 	}

tests/api/RestfulServiceTest.php

@@ -365,10 +365,11 @@ class RestfulServiceTest_Controller extends Controller implements TestOnly {
 	<body>$body</body>
 </test>
 XML;
-		$this->response->setBody($out);
+		$response = $this->getResponse();
-		$this->response->addHeader('Content-type', 'text/xml');
+		$response->setBody($out);
+		$response->addHeader('Content-type', 'text/xml');
 
-		return $this->response;
+		return $response;
 	}
 
 	public function invalid() {
@@ -390,11 +391,11 @@ XML;
 </test>
 XML;
 
-		$this->response->setBody($out);
+		$this->getResponse()->setBody($out);
-		$this->response->setStatusCode(400);
+		$this->getResponse()->setStatusCode(400);
-		$this->response->addHeader('Content-type', 'text/xml');
+		$this->getResponse()->addHeader('Content-type', 'text/xml');
 
-		return $this->response;
+		return $this->getResponse();
 	}
 
 	/**

tests/security/SecurityTest.php

@@ -79,7 +79,7 @@ class SecurityTest extends FunctionalTest {
 
 		// Controller that doesn't attempt redirections
 		$controller = new SecurityTest_NullController();
-		$controller->response = new SS_HTTPResponse();
+		$controller->setResponse(new SS_HTTPResponse());
 
 		Security::permissionFailure($controller, array('default' => 'Oops, not allowed'));
 		$this->assertEquals('Oops, not allowed', Session::get('Security.Message.message'));
@@ -104,12 +104,12 @@ class SecurityTest extends FunctionalTest {
 		Config::inst()->update('Security', 'default_message_set',
 			array('default' => 'default', 'alreadyLoggedIn' => 'You are already logged in!'));
 		Security::permissionFailure($controller);
-		$this->assertContains('You are already logged in!', $controller->response->getBody(),
+		$this->assertContains('You are already logged in!', $controller->getResponse()->getBody(),
 			'Custom permission failure message was ignored');
 
 		Security::permissionFailure($controller,
 			array('default' => 'default', 'alreadyLoggedIn' => 'One-off failure message'));
-		$this->assertContains('One-off failure message', $controller->response->getBody(),
+		$this->assertContains('One-off failure message', $controller->getResponse()->getBody(),
 			"Message set passed to Security::permissionFailure() didn't override Config values");
 
 		Config::unnest();