Refactor ExceptionRenderer.

The ablility to have methods per exception has been removed. That feature
wasn't DRY.

src/Error/ExceptionRenderer.php

@@ -58,20 +58,13 @@ class ExceptionRenderer {
 	public $controller = null;
 
 /**
- * template to render for Cake\Error\Exception
+ * Template to render for Cake\Error\Exception
  *
  * @var string
  */
 	public $template = '';
 
 /**
- * The method corresponding to the Exception this object is for.
- *
- * @var string
- */
-	public $method = '';
-
-/**
  * The exception being handled.
  *
  * @var Exception
@@ -86,40 +79,16 @@ class ExceptionRenderer {
  * @param \Exception $exception Exception
  */
 	public function __construct(\Exception $exception) {
-		$this->controller = $this->_getController($exception);
-
-		list(, $baseClass) = namespaceSplit(get_class($exception));
-		$baseClass = substr($baseClass, 0, -9);
-		$method = $template = Inflector::variable($baseClass);
-		$code = $exception->getCode();
-
-		$methodExists = method_exists($this, $method);
-
-		if ($exception instanceof Exception && !$methodExists) {
-			$method = '_cakeError';
-			if (empty($template) || $template === 'internalError') {
-				$template = 'error500';
-			}
-		} elseif ($exception instanceof \PDOException) {
-			$method = 'pdoError';
-			$template = 'pdo_error';
-			$code = 500;
-		} elseif (!$methodExists) {
-			$method = 'error500';
-			if ($code >= 400 && $code < 500) {
-				$method = 'error400';
+		if (!Configure::read('debug') && !($exception instanceof Error\HttpException)) {
+			$code = $this->_code($exception);
+			if ($code < 500) {
+				$exception = new Error\NotFoundException();
+			} else {
+				$exception = new Error\InternalErrorException();
 			}
 		}
 
-		$isNotDebug = !Configure::read('debug');
-		if ($isNotDebug && $method === '_cakeError') {
-			$method = 'error400';
-		}
-		if ($isNotDebug && $code == 500) {
-			$method = 'error500';
-		}
-		$this->template = $template;
-		$this->method = $method;
+		$this->controller = $this->_getController($exception);
 		$this->error = $exception;
 	}
 
@@ -164,95 +133,68 @@ class ExceptionRenderer {
  * @return void
  */
 	public function render() {
-		if ($this->method) {
-			call_user_func_array(array($this, $this->method), array($this->error));
-		}
-	}
-
-/**
- * Generic handler for the internal framework errors CakePHP can generate.
- *
- * @param \Cake\Error\Exception $error
- * @return void
- */
-	protected function _cakeError(Exception $error) {
+		$exception = $this->error;
+		$code = $this->_code($exception);
+		$template = $this->_template($exception, $code);
+		$message = $this->error->getMessage();
 		$url = $this->controller->request->here();
-		$code = ($error->getCode() >= 400 && $error->getCode() < 506) ? $error->getCode() : 500;
+
 		$this->controller->response->statusCode($code);
 		$this->controller->set(array(
-			'code' => $code,
-			'message' => h($error->getMessage()),
+			'message' => h($message),
 			'url' => h($url),
-			'error' => $error,
-			'_serialize' => array('code', 'message', 'url')
+			'error' => $exception,
+			'code' => $code,
+			'_serialize' => array('message', 'url', 'code')
 		));
-		$this->controller->set($error->getAttributes());
-		$this->_outputMessage($this->template);
-	}
 
-/**
- * Convenience method to display a 400 series page.
- *
- * @param Exception $error
- * @return void
- */
-	public function error400($error) {
-		$message = $error->getMessage();
-		if (!Configure::read('debug') && $error instanceof Exception) {
-			$message = __d('cake', 'Not Found');
+		if ($exception instanceof Error\Exception && Configure::read('debug')) {
+			$this->controller->set($this->error->getAttributes());
 		}
-		$url = $this->controller->request->here();
-		$this->controller->response->statusCode($error->getCode());
-		$this->controller->set(array(
-			'message' => h($message),
-			'url' => h($url),
-			'error' => $error,
-			'_serialize' => array('message', 'url')
-		));
-		$this->_outputMessage('error400');
+
+		$this->_outputMessage($template);
 	}
 
 /**
- * Convenience method to display a 500 page.
- *
- * @param \Exception $error
- * @return void
+ * Get template for rendering exception info
+ * @param \Exception $exception
+ * @param int $code Error code
+ * @return string Template name
  */
-	public function error500($error) {
-		$message = $error->getMessage();
-		if (!Configure::read('debug')) {
-			$message = __d('cake', 'An Internal Error Has Occurred.');
+	protected function _template(\Exception $exception, $code) {
+		list(, $baseClass) = namespaceSplit(get_class($exception));
+		$baseClass = substr($baseClass, 0, -9);
+		$template = Inflector::variable($baseClass);
+
+		if (empty($template) || $template === 'internalError') {
+			$template = 'error500';
 		}
-		$url = $this->controller->request->here();
-		$code = ($error->getCode() > 500 && $error->getCode() < 506) ? $error->getCode() : 500;
-		$this->controller->response->statusCode($code);
-		$this->controller->set(array(
-			'message' => h($message),
-			'url' => h($url),
-			'error' => $error,
-			'_serialize' => array('message', 'url')
-		));
-		$this->_outputMessage('error500');
+
+		if ($exception instanceof \PDOException) {
+			$template = 'pdo_error';
+		} elseif ($exception instanceof Error\HttpException) {
+			$template = 'error500';
+			if ($code < 500) {
+				$template = 'error400';
+			}
+		}
+
+		return $this->template = $template;
 	}
 
 /**
- * Convenience method to display a PDOException.
+ * Get an error code value within range 400 to 506
  *
- * @param \PDOException $error
- * @return void
+ * @param \Exception $exception Exception
+ * @return int Error code value within range 400 to 506
  */
-	public function pdoError(\PDOException $error) {
-		$url = $this->controller->request->here();
+	protected function _code(\Exception $exception) {
 		$code = 500;
-		$this->controller->response->statusCode($code);
-		$this->controller->set(array(
-			'code' => $code,
-			'message' => h($error->getMessage()),
-			'url' => h($url),
-			'error' => $error,
-			'_serialize' => array('code', 'message', 'url', 'error')
-		));
-		$this->_outputMessage($this->template);
+		$errorCode = $exception->getCode();
+		if ($errorCode >= 400 && $errorCode < 506) {
+			$code = $errorCode;
+		}
+		return $code;
 	}
 
 /**

tests/TestCase/Error/ExceptionRendererTest.php

@@ -107,16 +107,6 @@ class TestErrorController extends Controller {
  *
  */
 class MyCustomExceptionRenderer extends ExceptionRenderer {
-
-/**
- * custom error message type.
- *
- * @return void
- */
-	public function missingWidgetThing() {
-		echo 'widget thing is missing';
-	}
-
 }
 
 /**
@@ -132,6 +122,9 @@ class MissingWidgetThingException extends Error\NotFoundException {
  */
 class ExceptionRendererTest extends TestCase {
 
+/**
+ * @var boolean
+ */
 	protected $_restoreError = false;
 
 /**
@@ -173,44 +166,6 @@ class ExceptionRendererTest extends TestCase {
 	}
 
 /**
- * test that methods declared in an ExceptionRenderer subclass are not converted
- * into error400 when debug > 0
- *
- * @return void
- */
-	public function testSubclassMethodsNotBeingConvertedToError() {
-		Configure::write('debug', true);
-
-		$exception = new MissingWidgetThingException('Widget not found');
-		$ExceptionRenderer = $this->_mockResponse(new MyCustomExceptionRenderer($exception));
-
-		ob_start();
-		$ExceptionRenderer->render();
-		$result = ob_get_clean();
-
-		$this->assertEquals('widget thing is missing', $result);
-	}
-
-/**
- * test that subclass methods are not converted when debug = 0
- *
- * @return void
- */
-	public function testSubclassMethodsNotBeingConvertedDebug0() {
-		Configure::write('debug', false);
-		$exception = new MissingWidgetThingException('Widget not found');
-		$ExceptionRenderer = $this->_mockResponse(new MyCustomExceptionRenderer($exception));
-
-		$this->assertEquals('missingWidgetThing', $ExceptionRenderer->method);
-
-		ob_start();
-		$ExceptionRenderer->render();
-		$result = ob_get_clean();
-
-		$this->assertEquals('widget thing is missing', $result, 'Method declared in subclass converted to error400');
-	}
-
-/**
  * test that ExceptionRenderer subclasses properly convert framework errors.
  *
  * @return void
@@ -221,8 +176,6 @@ class ExceptionRendererTest extends TestCase {
 		$exception = new MissingControllerException('PostsController');
 		$ExceptionRenderer = $this->_mockResponse(new MyCustomExceptionRenderer($exception));
 
-		$this->assertEquals('error400', $ExceptionRenderer->method);
-
 		ob_start();
 		$ExceptionRenderer->render();
 		$result = ob_get_clean();
@@ -240,23 +193,21 @@ class ExceptionRendererTest extends TestCase {
 		$ExceptionRenderer = new ExceptionRenderer($exception);
 
 		$this->assertInstanceOf('Cake\Controller\ErrorController', $ExceptionRenderer->controller);
-		$this->assertEquals('error400', $ExceptionRenderer->method);
 		$this->assertEquals($exception, $ExceptionRenderer->error);
 	}
 
 /**
- * test that method gets coerced when debug = 0
+ * test that exception gets coerced when debug = 0
  *
  * @return void
  */
-	public function testErrorMethodCoercion() {
+	public function testExceptionCoercion() {
 		Configure::write('debug', false);
 		$exception = new MissingActionException('Page not found');
 		$ExceptionRenderer = new ExceptionRenderer($exception);
 
 		$this->assertInstanceOf('Cake\Controller\ErrorController', $ExceptionRenderer->controller);
-		$this->assertEquals('error400', $ExceptionRenderer->method);
-		$this->assertEquals($exception, $ExceptionRenderer->error);
+		$this->assertTrue($ExceptionRenderer->error instanceof Error\NotFoundException);
 	}
 
 /**
@@ -291,7 +242,6 @@ class ExceptionRendererTest extends TestCase {
 		$result = ob_get_clean();
 
 		$this->assertFalse(method_exists($ExceptionRenderer, 'missingWidgetThing'), 'no method should exist.');
-		$this->assertEquals('error400', $ExceptionRenderer->method, 'incorrect method coercion.');
 		$this->assertContains('coding fail', $result, 'Text should show up.');
 	}
 
@@ -312,7 +262,6 @@ class ExceptionRendererTest extends TestCase {
 		$ExceptionRenderer->render();
 		$result = ob_get_clean();
 
-		$this->assertEquals('error500', $ExceptionRenderer->method, 'incorrect method coercion.');
 		$this->assertContains('foul ball.', $result, 'Text should show up as its debug mode.');
 	}
 
@@ -335,7 +284,6 @@ class ExceptionRendererTest extends TestCase {
 		$ExceptionRenderer->render();
 		$result = ob_get_clean();
 
-		$this->assertEquals('error500', $ExceptionRenderer->method, 'incorrect method coercion.');
 		$this->assertNotContains('foul ball.', $result, 'Text should no show up.');
 		$this->assertContains('Internal Error', $result, 'Generic message only.');
 	}
@@ -355,7 +303,6 @@ class ExceptionRendererTest extends TestCase {
 		$ExceptionRenderer->render();
 		$result = ob_get_clean();
 
-		$this->assertEquals('error500', $ExceptionRenderer->method, 'incorrect method coercion.');
 		$this->assertContains('foul ball.', $result, 'Text should show up as its debug mode.');
 	}
 
@@ -480,12 +427,13 @@ class ExceptionRendererTest extends TestCase {
 			'prefix' => '',
 			'plugin' => '',
 		));
-		$ExceptionRenderer = $this->_mockResponse(new ExceptionRenderer($exception));
+		$ExceptionRenderer = $this->_mockResponse(new MyCustomExceptionRenderer($exception));
 
 		ob_start();
 		$ExceptionRenderer->render();
 		$result = ob_get_clean();
 
+		$this->assertEquals('missingController', $ExceptionRenderer->template);
 		$this->assertRegExp('/<h2>Missing Controller<\/h2>/', $result);
 		$this->assertRegExp('/<em>PostsController<\/em>/', $result);
 	}