From a158d437f95f623c0abe6319aba4b2e3ea88a786 Mon Sep 17 00:00:00 2001
From: mark_story
Date: Wed, 8 Nov 2017 21:42:34 -0500
Subject: [PATCH] Fix failing deprecated method use in ExceptionRenderer tests.
Fix a bunch of related things as they were in the call stack as well.
---
.../Component/RequestHandlerComponent.php | 9 +-
src/Core/Exception/Exception.php | 11 +-
src/Error/ExceptionRenderer.php | 21 +-
src/Http/Response.php | 28 ++-
src/Template/Error/missing_template.ctp | 6 +-
src/Template/Error/pdo_error.ctp | 2 +-
.../TestCase/Error/ExceptionRendererTest.php | 223 ++++++------------
7 files changed, 122 insertions(+), 178 deletions(-)
diff --git a/src/Controller/Component/RequestHandlerComponent.php b/src/Controller/Component/RequestHandlerComponent.php
index 3eb557509c8..f0cb4086c6a 100644
--- a/src/Controller/Component/RequestHandlerComponent.php
+++ b/src/Controller/Component/RequestHandlerComponent.php
@@ -331,7 +331,7 @@ public function beforeRender(Event $event)
if ($this->ext && $isRecognized) {
$this->renderAs($controller, $this->ext);
} else {
- $response->charset(Configure::read('App.encoding'));
+ $controller->response = $response->withCharset(Configure::read('App.encoding'));
}
if ($this->_config['checkHttpCache'] &&
@@ -652,14 +652,15 @@ public function respondAs($type, array $options = [])
return false;
}
if (!$request->getParam('requested')) {
- $response->type($cType);
+ $response = $response->withType($cType);
}
if (!empty($options['charset'])) {
- $response->charset($options['charset']);
+ $response = $response->withCharset($options['charset']);
}
if (!empty($options['attachment'])) {
- $response->download($options['attachment']);
+ $response = $response->withDownload($options['attachment']);
}
+ $controller->response = $response;
return true;
}
diff --git a/src/Core/Exception/Exception.php b/src/Core/Exception/Exception.php
index 6baa16ddd55..6553991a7ad 100644
--- a/src/Core/Exception/Exception.php
+++ b/src/Core/Exception/Exception.php
@@ -86,11 +86,11 @@ public function getAttributes()
/**
* Get/set the response header to be used
*
- * See also Cake\Http\Response::header()
+ * See also Cake\Http\Response::withHeader()
*
* @param string|array|null $header An array of header strings or a single header string
* - an associative array of "header name" => "header value"
- * - an array of string headers is also accepted
+ * - an array of string headers is also accepted (deprecated)
* @param string|null $value The header value.
* @return array
*/
@@ -100,6 +100,13 @@ public function responseHeader($header = null, $value = null)
return $this->_responseHeaders;
}
if (is_array($header)) {
+ if (isset($header[0])) {
+ deprecationWarning(
+ 'Passing a list string headers to Exception::responseHeader() is deprecated. ' .
+ 'Use an associative array instead.'
+ );
+ }
+
return $this->_responseHeaders = $header;
}
$this->_responseHeaders = [$header => $value];
diff --git a/src/Error/ExceptionRenderer.php b/src/Error/ExceptionRenderer.php
index 08596d09df6..cac8cfecc8e 100644
--- a/src/Error/ExceptionRenderer.php
+++ b/src/Error/ExceptionRenderer.php
@@ -21,7 +21,7 @@
use Cake\Core\Exception\MissingPluginException;
use Cake\Event\Event;
use Cake\Http\Response;
-use Cake\Http\ServerRequest;
+use Cake\Http\ServerRequestFactory;
use Cake\Network\Exception\HttpException;
use Cake\Routing\DispatcherFactory;
use Cake\Routing\Router;
@@ -115,7 +115,7 @@ protected function _unwrap($exception)
protected function _getController()
{
if (!$request = Router::getRequest(true)) {
- $request = ServerRequest::createFromGlobals();
+ $request = ServerRequestFactory::fromGlobals();
}
$response = new Response();
$controller = null;
@@ -169,11 +169,15 @@ public function render()
$message = $this->_message($exception, $code);
$url = $this->controller->request->getRequestTarget();
+ $response = $this->controller->response;
if ($exception instanceof CakeException) {
- $this->controller->response->header($exception->responseHeader());
+ foreach ((array)$exception->responseHeader() as $key => $value) {
+ $response = $response->withHeader($key, $value);
+ }
}
- $this->controller->response->statusCode($code);
+ $response = $response->withStatus($code);
+
$viewVars = [
'message' => $message,
'url' => h($url),
@@ -197,6 +201,7 @@ public function render()
$this->controller->set($unwrapped->getAttributes());
}
+ $this->controller->response = $response;
return $this->_outputMessage($template);
}
@@ -212,8 +217,7 @@ protected function _customMethod($method, $exception)
$result = call_user_func([$this, $method], $exception);
$this->_shutdown();
if (is_string($result)) {
- $this->controller->response->body($result);
- $result = $this->controller->response;
+ $result = $this->controller->response->withStringBody($result);
}
return $result;
@@ -361,8 +365,9 @@ protected function _outputMessageSafe($template)
->setTemplatePath('Error');
$view = $this->controller->createView('View');
- $this->controller->response->body($view->render($template, 'error'));
- $this->controller->response->type('html');
+ $this->controller->response = $this->controller->response
+ ->withType('html')
+ ->withStringBody($view->render($template, 'error'));
return $this->controller->response;
}
diff --git a/src/Http/Response.php b/src/Http/Response.php
index a8932858941..87c174efc23 100644
--- a/src/Http/Response.php
+++ b/src/Http/Response.php
@@ -484,9 +484,7 @@ protected function _createStream()
*/
public function send()
{
- deprecationWarning(
- 'Will be removed in 4.0.0'
- );
+ deprecationWarning('Response::send() will be removed in 4.0.0');
if ($this->hasHeader('Location') && $this->_status === 200) {
$this->statusCode(302);
@@ -1625,7 +1623,7 @@ public function modified($time = null)
{
deprecationWarning(
'Response::modified() is deprecated. ' .
- 'Use withModified() instead.'
+ 'Use withModified() or getHeaderLine("Last-Modified") instead.'
);
if ($time !== null) {
@@ -1668,12 +1666,14 @@ public function withModified($time)
* setting the status code to "304 Not Modified" and removing all
* conflicting headers
*
+ * *Warning* This method mutates the response in-place and should be avoided.
+ *
* @return void
*/
public function notModified()
{
- $this->statusCode(304);
- $this->body('');
+ $this->_status = 304;
+ $this->_createStream();
$remove = [
'Allow',
@@ -1790,7 +1790,7 @@ public function etag($hash = null, $weak = false)
{
deprecationWarning(
'Response::etag() is deprecated. ' .
- 'Use withEtag() instead.'
+ 'Use withEtag() or getHeaderLine("Etag") instead.'
);
if ($hash !== null) {
@@ -1976,18 +1976,22 @@ public function withLength($bytes)
* the Last-Modified etag response header before calling this method. Otherwise
* a comparison will not be possible.
*
+ * *Warning* This method mutates the response in-place and should be avoided.
+ *
* @param \Cake\Http\ServerRequest $request Request object
* @return bool Whether the response was marked as not modified or not.
*/
public function checkNotModified(ServerRequest $request)
{
- $etags = preg_split('/\s*,\s*/', (string)$request->header('If-None-Match'), 0, PREG_SPLIT_NO_EMPTY);
- $modifiedSince = $request->header('If-Modified-Since');
- if ($responseTag = $this->etag()) {
+ $etags = preg_split('/\s*,\s*/', (string)$request->getHeaderLine('If-None-Match'), 0, PREG_SPLIT_NO_EMPTY);
+ $responseTag = $this->getHeaderLine('Etag');
+ if ($responseTag) {
$etagMatches = in_array('*', $etags) || in_array($responseTag, $etags);
}
- if ($modifiedSince) {
- $timeMatches = strtotime($this->modified()) === strtotime($modifiedSince);
+
+ $modifiedSince = $request->getHeaderLine('If-Modified-Since');
+ if ($modifiedSince && $this->hasHeader('Last-Modified')) {
+ $timeMatches = strtotime($this->getHeaderLine('Last-Modifed')) === strtotime($modifiedSince);
}
$checks = compact('etagMatches', 'timeMatches');
if (empty($checks)) {
diff --git a/src/Template/Error/missing_template.ctp b/src/Template/Error/missing_template.ctp
index 0d4f5a38085..d2529f67c73 100644
--- a/src/Template/Error/missing_template.ctp
+++ b/src/Template/Error/missing_template.ctp
@@ -28,7 +28,11 @@ $this->start('subheading');
= sprintf('The template %s was not found.', h($file)); ?>
Error:
- = sprintf('The view for %sController::%s() was not found.', h(Inflector::camelize($this->request->controller)), h($this->request->action)); ?>
+ = sprintf(
+ 'The view for %sController::%s() was not found.',
+ h(Inflector::camelize($this->request->getParam('controller'))),
+ h($this->request->getParam('action'))
+ ); ?>
end() ?>
diff --git a/src/Template/Error/pdo_error.ctp b/src/Template/Error/pdo_error.ctp
index df76bef9fd4..154fb25388c 100644
--- a/src/Template/Error/pdo_error.ctp
+++ b/src/Template/Error/pdo_error.ctp
@@ -14,7 +14,7 @@
*/
use Cake\Error\Debugger;
-$this->layout = 'dev_error';
+$this->setLayout('dev_error');
$this->assign('title', 'Database Error');
$this->assign('templateName', 'pdo_error.ctp');
diff --git a/tests/TestCase/Error/ExceptionRendererTest.php b/tests/TestCase/Error/ExceptionRendererTest.php
index f7adbd336e1..87e833ea82c 100644
--- a/tests/TestCase/Error/ExceptionRendererTest.php
+++ b/tests/TestCase/Error/ExceptionRendererTest.php
@@ -184,20 +184,6 @@ public function tearDown()
}
}
- /**
- * Mocks out the response on the ExceptionRenderer object so headers aren't modified.
- *
- * @return void
- */
- protected function _mockResponse($error)
- {
- $error->controller->response = $this->getMockBuilder('Cake\Http\Response')
- ->setMethods(['_sendHeader'])
- ->getMock();
-
- return $error;
- }
-
/**
* test that methods declared in an ExceptionRenderer subclass are not converted
* into error400 when debug > 0
@@ -207,11 +193,11 @@ protected function _mockResponse($error)
public function testSubclassMethodsNotBeingConvertedToError()
{
$exception = new MissingWidgetThingException('Widget not found');
- $ExceptionRenderer = $this->_mockResponse(new MyCustomExceptionRenderer($exception));
+ $ExceptionRenderer = new MyCustomExceptionRenderer($exception);
$result = $ExceptionRenderer->render();
- $this->assertEquals('widget thing is missing', $result->body());
+ $this->assertEquals('widget thing is missing', (string)$result->getBody());
}
/**
@@ -223,14 +209,14 @@ public function testSubclassMethodsNotBeingConvertedDebug0()
{
Configure::write('debug', false);
$exception = new MissingWidgetThingException('Widget not found');
- $ExceptionRenderer = $this->_mockResponse(new MyCustomExceptionRenderer($exception));
+ $ExceptionRenderer = new MyCustomExceptionRenderer($exception);
$result = $ExceptionRenderer->render();
$this->assertEquals('missingWidgetThing', $ExceptionRenderer->method);
$this->assertEquals(
'widget thing is missing',
- $result->body(),
+ (string)$result->getBody(),
'Method declared in subclass converted to error400'
);
}
@@ -245,13 +231,13 @@ public function testSubclassConvertingFrameworkErrors()
Configure::write('debug', false);
$exception = new MissingControllerException('PostsController');
- $ExceptionRenderer = $this->_mockResponse(new MyCustomExceptionRenderer($exception));
+ $ExceptionRenderer = new MyCustomExceptionRenderer($exception);
$result = $ExceptionRenderer->render();
$this->assertRegExp(
'/Not Found/',
- $result->body(),
+ (string)$result->getBody(),
'Method declared in error handler not converted to error400. %s'
);
}
@@ -279,12 +265,12 @@ public function testExceptionMessageCoercion()
{
Configure::write('debug', false);
$exception = new MissingActionException('Secret info not to be leaked');
- $ExceptionRenderer = $this->_mockResponse(new ExceptionRenderer($exception));
+ $ExceptionRenderer = new ExceptionRenderer($exception);
$this->assertInstanceOf('Cake\Controller\ErrorController', $ExceptionRenderer->controller);
$this->assertEquals($exception, $ExceptionRenderer->error);
- $result = $ExceptionRenderer->render()->body();
+ $result = (string)$ExceptionRenderer->render()->getBody();
$this->assertEquals('error400', $ExceptionRenderer->template);
$this->assertContains('Not Found', $result);
@@ -300,10 +286,10 @@ public function testCakeErrorHelpersNotLost()
{
static::setAppNamespace();
$exception = new SocketException('socket exception');
- $renderer = $this->_mockResponse(new \TestApp\Error\TestAppsExceptionRenderer($exception));
+ $renderer = new \TestApp\Error\TestAppsExceptionRenderer($exception);
$result = $renderer->render();
- $this->assertContains('peeled', $result->body());
+ $this->assertContains('peeled', (string)$result->getBody());
}
/**
@@ -315,15 +301,11 @@ public function testUnknownExceptionTypeWithExceptionThatHasA400Code()
{
$exception = new MissingWidgetThingException('coding fail.');
$ExceptionRenderer = new ExceptionRenderer($exception);
- $ExceptionRenderer->controller->response = $this->getMockBuilder('Cake\Http\Response')
- ->setMethods(['statusCode', '_sendHeader'])
- ->getMock();
- $ExceptionRenderer->controller->response->expects($this->once())->method('statusCode')->with(404);
-
- $result = $ExceptionRenderer->render();
+ $response = $ExceptionRenderer->render();
+ $this->assertEquals(404, $response->getStatusCode());
$this->assertFalse(method_exists($ExceptionRenderer, 'missingWidgetThing'), 'no method should exist.');
- $this->assertContains('coding fail', $result->body(), 'Text should show up.');
+ $this->assertContains('coding fail', (string)$response->getBody(), 'Text should show up.');
}
/**
@@ -335,16 +317,10 @@ public function testUnknownExceptionTypeWithNoCodeIsA500()
{
$exception = new \OutOfBoundsException('foul ball.');
$ExceptionRenderer = new ExceptionRenderer($exception);
- $ExceptionRenderer->controller->response = $this->getMockBuilder('Cake\Http\Response')
- ->setMethods(['statusCode', '_sendHeader'])
- ->getMock();
- $ExceptionRenderer->controller->response->expects($this->once())
- ->method('statusCode')
- ->with(500);
-
$result = $ExceptionRenderer->render();
- $this->assertContains('foul ball.', $result->body(), 'Text should show up as its debug mode.');
+ $this->assertEquals(500, $result->getStatusCode());
+ $this->assertContains('foul ball.', (string)$result->getBody(), 'Text should show up as its debug mode.');
}
/**
@@ -358,15 +334,11 @@ public function testUnknownExceptionInProduction()
$exception = new \OutOfBoundsException('foul ball.');
$ExceptionRenderer = new ExceptionRenderer($exception);
- $ExceptionRenderer->controller->response = $this->getMockBuilder('Cake\Http\Response')
- ->setMethods(['statusCode', '_sendHeader'])
- ->getMock();
- $ExceptionRenderer->controller->response->expects($this->once())
- ->method('statusCode')
- ->with(500);
- $result = $ExceptionRenderer->render()->body();
+ $response = $ExceptionRenderer->render();
+ $result = (string)$response->getBody();
+ $this->assertEquals(500, $response->getStatusCode());
$this->assertNotContains('foul ball.', $result, 'Text should no show up.');
$this->assertContains('Internal Error', $result, 'Generic message only.');
}
@@ -380,14 +352,11 @@ public function testUnknownExceptionTypeWithCodeHigherThan500()
{
$exception = new \OutOfBoundsException('foul ball.', 501);
$ExceptionRenderer = new ExceptionRenderer($exception);
- $ExceptionRenderer->controller->response = $this->getMockBuilder('Cake\Http\Response')
- ->setMethods(['statusCode', '_sendHeader'])
- ->getMock();
- $ExceptionRenderer->controller->response->expects($this->once())->method('statusCode')->with(501);
-
- $result = $ExceptionRenderer->render();
+ $response = $ExceptionRenderer->render();
+ $result = (string)$response->getBody();
- $this->assertContains('foul ball.', $result->body(), 'Text should show up as its debug mode.');
+ $this->assertEquals(501, $response->getStatusCode());
+ $this->assertContains('foul ball.', $result, 'Text should show up as its debug mode.');
}
/**
@@ -404,13 +373,11 @@ public function testError400()
$exception = new NotFoundException('Custom message');
$ExceptionRenderer = new ExceptionRenderer($exception);
- $ExceptionRenderer->controller->response = $this->getMockBuilder('Cake\Http\Response')
- ->setMethods(['statusCode', '_sendHeader'])
- ->getMock();
- $ExceptionRenderer->controller->response->expects($this->once())->method('statusCode')->with(404);
- $result = $ExceptionRenderer->render()->body();
+ $response = $ExceptionRenderer->render();
+ $result = (string)$response->getBody();
+ $this->assertEquals(404, $response->getStatusCode());
$this->assertContains('Custom message
', $result);
$this->assertRegExp("/'.*?\/posts\/view\/1000'<\/strong>/", $result);
}
@@ -432,12 +399,9 @@ public function testError400AsJson()
$exception = new NotFoundException('Custom message');
$exceptionLine = __LINE__ - 1;
$ExceptionRenderer = new ExceptionRenderer($exception);
- $ExceptionRenderer->controller->response = $this->getMockBuilder('Cake\Network\Response')
- ->setMethods(['statusCode', '_sendHeader'])
- ->getMock();
- $ExceptionRenderer->controller->response->expects($this->once())->method('statusCode')->with(404);
- $result = $ExceptionRenderer->render()->body();
+ $response = $ExceptionRenderer->render();
+ $result = (string)$response->getBody();
$expected = [
'message' => 'Custom message',
'url' => '/posts/view/1000?sort=title&direction=desc',
@@ -445,8 +409,8 @@ public function testError400AsJson()
'file' => __FILE__,
'line' => $exceptionLine
];
-
$this->assertEquals($expected, json_decode($result, true));
+ $this->assertEquals(404, $response->getStatusCode());
}
/**
@@ -459,16 +423,16 @@ public function testerror400OnlyChangingCakeException()
Configure::write('debug', false);
$exception = new NotFoundException('Custom message');
- $ExceptionRenderer = $this->_mockResponse(new ExceptionRenderer($exception));
+ $ExceptionRenderer = new ExceptionRenderer($exception);
$result = $ExceptionRenderer->render();
- $this->assertContains('Custom message', $result->body());
+ $this->assertContains('Custom message', (string)$result->getBody());
$exception = new MissingActionException(['controller' => 'PostsController', 'action' => 'index']);
- $ExceptionRenderer = $this->_mockResponse(new ExceptionRenderer($exception));
+ $ExceptionRenderer = new ExceptionRenderer($exception);
$result = $ExceptionRenderer->render();
- $this->assertContains('Not Found', $result->body());
+ $this->assertContains('Not Found', (string)$result->getBody());
}
/**
@@ -484,9 +448,9 @@ public function testError400NoInjection()
Router::setRequestInfo($request);
$exception = new NotFoundException('Custom message');
- $ExceptionRenderer = $this->_mockResponse(new ExceptionRenderer($exception));
+ $ExceptionRenderer = new ExceptionRenderer($exception);
- $result = $ExceptionRenderer->render()->body();
+ $result = (string)$ExceptionRenderer->render()->getBody();
$this->assertNotContains('', $result);
@@ -501,14 +465,12 @@ public function testError500Message()
{
$exception = new InternalErrorException('An Internal Error Has Occurred.');
$ExceptionRenderer = new ExceptionRenderer($exception);
- $ExceptionRenderer->controller->response = $this->getMockBuilder('Cake\Http\Response')
- ->setMethods(['statusCode', '_sendHeader'])
- ->getMock();
- $ExceptionRenderer->controller->response->expects($this->once())->method('statusCode')->with(500);
- $result = $ExceptionRenderer->render();
- $this->assertContains('An Internal Error Has Occurred.
', $result->body());
- $this->assertContains('An Internal Error Has Occurred.