Merge pull request #35 from kenjis/codeingiter-class

Refactor 404 logic

Author: lonnieezell

system/CodeIgniter.php

@@ -157,15 +157,22 @@ public function run()
 		//--------------------------------------------------------------------
 		Hooks::trigger('pre_controller');
 
-		$this->startController();
+		try
+		{
+			$this->startController();
 
-		//--------------------------------------------------------------------
-		// Is there a "post_controller" hook?
-		//--------------------------------------------------------------------
-		Hooks::trigger('post_controller');
+			//--------------------------------------------------------------------
+			// Is there a "post_controller" hook?
+			//--------------------------------------------------------------------
+			Hooks::trigger('post_controller');
 
-		$this->gatherOutput();
-		$this->sendResponse();
+			$this->gatherOutput();
+			$this->sendResponse();
+		}
+		catch (PageNotFoundException $e)
+		{
+			$this->display404errors($e);
+		}
 
 		//--------------------------------------------------------------------
 		// Is there a post-system hook?
@@ -300,11 +307,15 @@ protected function loadComposerAutoloader()
 	 */
 	protected function getRequestObject()
 	{
-		$this->request = is_cli()
-			? Services::clirequest($this->config, true)
-			: Services::request($this->config, true);
-
-		$this->request->setProtocolVersion($_SERVER['SERVER_PROTOCOL']);
+		if (is_cli())
+		{
+			$this->request = Services::clirequest($this->config, true);
+		}
+		else
+		{
+			$this->request = Services::request($this->config, true);
+			$this->request->setProtocolVersion($_SERVER['SERVER_PROTOCOL']);
+		}
 	}
 
 	//--------------------------------------------------------------------
@@ -316,7 +327,10 @@ protected function getRequestObject()
 	protected function getResponseObject()
 	{
 		$this->response = Services::response($this->config, true);
-		$this->response->setProtocolVersion($this->request->getProtocolVersion());
+		if ( ! is_cli())
+		{
+			$this->response->setProtocolVersion($this->request->getProtocolVersion());
+		}
 
 		// Assume success until proven otherwise.
 		$this->response->setStatusCode(200);
@@ -415,8 +429,6 @@ protected function startController()
 		$this->benchmark->start('controller');
 		$this->benchmark->start('controller_constructor');
 
-		$e404 = false;
-
 		// Is it routed to a Closure?
 		if (is_callable($this->controller))
 		{
@@ -426,92 +438,100 @@ protected function startController()
 		{
 			if (empty($this->controller))
 			{
-				$e404 = true;
+				throw new PageNotFoundException('Controller is empty.');
 			}
 			else
 			{
 				// Try to autoload the class
 				if ( ! class_exists($this->controller, true) || $this->method[0] === '_')
 				{
-					$e404 = true;
+					throw new PageNotFoundException('Controller or its method is not found.');
 				}
 				else if ( ! method_exists($this->controller, '_remap') &&
 					! is_callable([$this->controller, $this->method], false)
 				)
 				{
-					$e404 = true;
+					throw new PageNotFoundException('Controller method is not found.');
 				}
 			}
+		}
 
-			// Is there a 404 Override available?
-			if ($e404 && $override = $this->router->get404Override())
-			{
-				if ($override instanceof \Closure)
-				{
-					echo $override();
-				}
-				else if (is_array($override))
-				{
-					$this->controller = $override[0];
-					$this->method = $override[1];
+		$this->createControllerAndRun();
+	}
 
-					unset($override);
-				}
+	protected function createControllerAndRun()
+	{
+		$class = new $this->controller($this->request, $this->response);
 
-				$e404 = false;
-			}
+		$this->benchmark->stop('controller_constructor');
 
-			// Display 404 Errors
-			if ($e404)
-			{
-				$this->response->setStatusCode(404);
+		//--------------------------------------------------------------------
+		// Is there a "post_controller_constructor" hook?
+		//--------------------------------------------------------------------
+		Hooks::trigger('post_controller_constructor');
 
-				if (ob_get_level() > 0)
-				{
-					ob_end_flush();
-				}
-				ob_start();
+		if (method_exists($class, '_remap'))
+		{
+			$class->_remap($this->method, ...$this->router->params());
+		}
+		else
+		{
+			$class->{$this->method}(...$this->router->params());
+		}
 
-				// Show the 404 error page
-				if (is_cli())
-				{
-					require APPPATH.'Views/errors/cli/error_404.php';
-				}
-				else
-				{
-					require APPPATH.'Views/errors/html/error_404.php';
-				}
+		$this->benchmark->stop('controller');
+	}
 
-				$buffer = ob_get_contents();
-				ob_end_clean();
+	protected function display404errors(PageNotFoundException $e)
+	{
+		// Is there a 404 Override available?
+		if ($override = $this->router->get404Override())
+		{
+			if ($override instanceof \Closure)
+			{
+				echo $override();
+			}
+			else if (is_array($override))
+			{
+				$this->controller = $override[0];
+				$this->method = $override[1];
 
-				echo $buffer;
-				exit(EXIT_UNKNOWN_FILE);    // Unknown file
+				unset($override);
 			}
 
-			if ( ! $e404 && ! isset($override))
-			{
-				$class = new $this->controller($this->request, $this->response);
+			$this->createControllerAndRun();
+			$this->gatherOutput();
+			$this->sendResponse();
+			return;
+		}
 
-				$this->benchmark->stop('controller_constructor');
+		// Display 404 Errors
+		$this->response->setStatusCode(404);
 
-				//--------------------------------------------------------------------
-				// Is there a "post_controller_constructor" hook?
-				//--------------------------------------------------------------------
-				Hooks::trigger('post_controller_constructor');
+		if (ob_get_level() > 0)
+		{
+			ob_end_flush();
+		}
+		ob_start();
 
-				if (method_exists($class, '_remap'))
-				{
-					$class->_remap($this->method, ...$this->router->params());
-				}
-				else
-				{
-					$class->{$this->method}(...$this->router->params());
-				}
-			}
+		$heading = 'Page Not Found';
+		$message = $e->getMessage();
+
+		// Show the 404 error page
+		if (is_cli())
+		{
+			require APPPATH.'Views/errors/cli/error_404.php';
+		}
+		else
+		{
+			require APPPATH.'Views/errors/html/error_404.php';
 		}
 
-		$this->benchmark->stop('controller');
+		$buffer = ob_get_contents();
+		ob_end_clean();
+
+		echo $buffer;
+		exit(EXIT_UNKNOWN_FILE);    // Unknown file
 	}
 
 	//--------------------------------------------------------------------
@@ -532,7 +552,7 @@ protected function gatherOutput()
 		//--------------------------------------------------------------------
 		// Display the Debug Toolbar?
 		//--------------------------------------------------------------------
-		if (ENVIRONMENT != 'production' && $this->config->toolbarEnabled)
+		if ( ! is_cli() && ENVIRONMENT != 'production' && $this->config->toolbarEnabled)
 		{
 			$toolbar = Services::toolbar($this->config);
 			$this->output .= $toolbar->run($this->startTime, $totalTime,

system/HTTP/Message.php

@@ -330,7 +330,7 @@ public function prependHeader(string $name, $value): self
 	 */
 	public function getProtocolVersion(): string
 	{
-	    return $this->protocolVersion;
+		return $this->protocolVersion;
 	}
 
 	//--------------------------------------------------------------------