Refactoring and improving test coverage.

Signed-off-by: Jason Lewis <[email protected]>

Author: jasonlewis

src/Dingo/Api/Dispatcher.php

@@ -169,18 +169,6 @@ public function patch($uri, $parameters = [])
 		return $this->queueRequest('patch', $uri, $parameters);
 	}
 
-	/**
-	 * Perform API HEAD request.
-	 * 
-	 * @param  string  $uri
-	 * @param  array  $parameters
-	 * @return mixed
-	 */
-	public function head($uri, $parameters = [])
-	{
-		return $this->queueRequest('head', $uri, $parameters);
-	}
-
 	/**
 	 * Perform API DELETE request.
 	 * 
@@ -274,8 +262,6 @@ protected function dispatch(InternalRequest $request)
 	{
 		try
 		{
-			$this->router->enableApiRouting();
-
 			$response = $this->router->dispatch($request);
 
 			if ( ! $response->isOk())

src/Dingo/Api/Routing/ApiCollection.php

@@ -56,7 +56,7 @@ public function matches($request)
 		{
 			return true;
 		}
-		elseif ($this->option('prefix') and starts_with($request->getPathInfo(), "/{$this->option('prefix')}"))
+		elseif ($this->option('prefix') and starts_with($request->getPathInfo(), trim($this->option('prefix'), '/')))
 		{
 			return true;
 		}

src/Dingo/Api/Routing/Router.php

@@ -20,13 +20,6 @@ class Router extends \Illuminate\Routing\Router {
 	 */
 	protected $apiCollections = [];
 
-	/**
-	 * Indicates if newly added routes are API routes.
-	 * 
-	 * @var bool
-	 */
-	protected $apiRouting = false;
-
 	/**
 	 * The default API version.
 	 * 
@@ -48,6 +41,13 @@ class Router extends \Illuminate\Routing\Router {
 	 */
 	protected $exceptionHandler;
 
+	/**
+	 * Array of cached API requests.
+	 * 
+	 * @var array
+	 */
+	protected $cachedApiRequests = [];
+
 	/**
 	 * Register an API group.
 	 * 
@@ -62,21 +62,19 @@ public function api($options, Closure $callback)
 			throw new BadMethodCallException('Unable to register API without an API version.');
 		}
 
-		$this->enableApiRouting();
-
 		// Once we have the version from the options we'll check to see if an API
 		// collection already exists for this verison. If it doesn't then we'll
 		// create a new collection.
 		$version = $options['version'];
 
+		$options['api'] = true;
+
 		if ( ! isset($this->apiCollections[$version]))
 		{
 			$this->apiCollections[$version] = $this->newApiCollection($version, array_except($options, ['version']));
 		}
 
 		$this->group($options, $callback);
-
-		$this->disableApiRouting();
 	}
 
 	/**
@@ -107,36 +105,23 @@ public function dispatch(Request $request)
 			// If an exception is caught and we are currently routing an API request then
 			// we'll handle this exception by building a new response from it. This
 			// allows the API to gracefully handle its own exceptions.
-			if (($this->routingForApi() or $this->requestTargettingApi($request)) and ! $request instanceof InternalRequest)
+			if ($this->requestTargettingApi($request) and ! $request instanceof InternalRequest)
 			{
 				$response = $this->handleException($exception);
 			}
 
 			// If the request was an internal request then we will rethrow the exception
 			// so that developers can easily catch them and adjust ther esponse
-			// themselves. We also disable API routing so that the parent
-			// request isn't treated as an API request.
+			// themselves.
 			else
 			{
-				$this->disableApiRouting();
-
 				throw $exception;
 			}
 		}
 
-		if ($this->routingForApi() or $this->requestTargettingApi($request))
+		if ($this->requestTargettingApi($request))
 		{
 			$response = Response::makeFromExisting($response)->morph();
-
-			// If the request that was dispatched is an internal request then we need to
-			// disable the API routing so that the parent request is not treated in
-			// the same way. This prevents it from generating an Api Response for
-			// the parent request. Another internal request will still result
-			// in API routing being enabled.
-			if ($request instanceof InternalRequest)
-			{
-				$this->disableApiRouting();
-			}
 		}
 
 		return $response;
@@ -186,7 +171,7 @@ protected function addRoute($methods, $uri, $action)
 	{
 		$route = $this->createRoute($methods, $uri, $action);
 
-		if ($this->routingForApi())
+		if ($this->routingForApi($route))
 		{
 			$route->before('api');
 
@@ -226,7 +211,7 @@ protected function createRoute($methods, $uri, $action)
 		// see if the controller is one of the API controllers. If it is then we
 		// need to check if the method is protected and get any scopes
 		// associated with the method.
-		if ($this->routingForApi() and $this->routingToController($action))
+		if ($this->routingForApi($route) and $this->routingToController($action))
 		{
 			list ($class, $method) = explode('@', $route->getActionName());
 
@@ -317,7 +302,7 @@ protected function controllerMethodProtected($route, $controller, $method)
 	 */
 	protected function findRoute($request)
 	{
-		if ($this->routingForApi() or $this->requestTargettingApi($request))
+		if ($this->requestTargettingApi($request))
 		{
 			$version = $this->getRequestVersion($request);
 
@@ -345,12 +330,22 @@ protected function findRoute($request)
 	 */
 	public function requestTargettingApi($request)
 	{
-		if (empty($this->apiCollections)) return false;
+		if (empty($this->apiCollections))
+		{
+			return false;
+		}
 
-		return array_first($this->apiCollections, function($key, $value) use ($request)
+		if (in_array($request, $this->cachedApiRequests))
 		{
-			return $value->matches($request);
+			return true;
+		}
+
+		$collection = array_first($this->apiCollections, function($key, $collection) use ($request)
+		{
+			return $collection->matches($request);
 		}, false);
+
+		return $collection instanceof ApiCollection;
 	}
 
 	/**
@@ -388,37 +383,16 @@ public function getApiCollection($version)
 	}
 
 	/**
-	 * Determine if the current request is an API request.
+	 * Determine if a route is an API route.
 	 * 
+	 * @param  \Illuminate\Routing\Route
 	 * @return bool
 	 */
-	public function routingForApi()
-	{
-		return $this->apiRouting;
-	}
-
-	/**
-	 * Enable API request.
-	 * 
-	 * @return \Dingo\Api\Routing\Router
-	 */
-	public function enableApiRouting()
+	public function routingForApi($route)
 	{
-		$this->apiRouting = true;
-
-		return $this;
-	}
-
-	/**
-	 * Disable API request.
-	 * 
-	 * @return \Dingo\Api\Routing\Router
-	 */
-	public function disableApiRouting()
-	{
-		$this->apiRouting = false;
+		$action = $route->getAction();
 
-		return $this;
+		return isset($action['api']) and $action['api'] === true;
 	}
 
 	/**

tests/DispatcherTest.php

@@ -8,7 +8,9 @@ class DispatcherTest extends PHPUnit_Framework_TestCase {
 	public function setUp()
 	{
 		$this->request = new Illuminate\Http\Request;
-		$this->router = m::mock('Dingo\Api\Routing\Router');
+		$this->router = new Dingo\Api\Routing\Router(new Illuminate\Events\Dispatcher);
+		$this->router->setDefaultApiVersion('v1');
+		$this->router->setApiVendor('test');
 		$this->auth = m::mock('Dingo\Api\Authentication');
 		$this->dispatcher = new Dingo\Api\Dispatcher($this->request, $this->router, $this->auth);
 	}
@@ -22,46 +24,57 @@ public function tearDown()
 
 	public function testInternalRequests()
 	{
-		$this->router->shouldReceive('dispatch')->andReturn(new Dingo\Api\Http\Response('test', 200));
+		$this->router->api(['version' => 'v1'], function()
+		{
+			$this->router->get('test', function(){ return 'test'; });
+			$this->router->post('test', function(){ return 'test'; });
+			$this->router->put('test', function(){ return 'test'; });
+			$this->router->patch('test', function(){ return 'test'; });
+			$this->router->delete('test', function(){ return 'test'; });
+		});
 
-		$this->router->shouldReceive('getDefaultApiVersion')->times(6)->andReturn('v1');
-		$this->router->shouldReceive('getApiVendor')->times(6)->andReturn('testing');
-		$this->router->shouldReceive('getApiCollection')->times(6)->with('v1')->andReturn(new Dingo\Api\Routing\ApiCollection('v1', ['prefix' => 'api']));
-		$this->router->shouldReceive('enableApiRouting')->times(6);
-
-		$this->auth->shouldReceive('setUser')->times(6)->with(null);
+		$this->auth->shouldReceive('setUser')->times(5)->with(null);
 
 		$this->assertEquals('test', $this->dispatcher->get('test'));
 		$this->assertEquals('test', $this->dispatcher->post('test'));
 		$this->assertEquals('test', $this->dispatcher->put('test'));
 		$this->assertEquals('test', $this->dispatcher->patch('test'));
-		$this->assertEquals('test', $this->dispatcher->head('test'));
 		$this->assertEquals('test', $this->dispatcher->delete('test'));
 	}
 
 
 	public function testInternalRequestWithVersionAndParameters()
 	{
-		$this->router->shouldReceive('dispatch')->andReturn(new Dingo\Api\Http\Response('test', 200));
-
-		$this->router->shouldReceive('getApiVendor')->once()->andReturn('testing');
-		$this->router->shouldReceive('getApiCollection')->once()->with('v1')->andReturn(new Dingo\Api\Routing\ApiCollection('v1', ['prefix' => 'api']));
-		$this->router->shouldReceive('enableApiRouting')->once();
+		$this->router->api(['version' => 'v1'], function()
+		{
+			$this->router->get('test', function(){ return 'test'; });
+		});
 
 		$this->auth->shouldReceive('setUser')->once()->with(null);
 
 		$this->assertEquals('test', $this->dispatcher->version('v1')->with(['foo' => 'bar'])->get('test'));
 	}
 
 
-	public function testInternalRequestWithPrefixAndDomain()
+	public function testInternalRequestWithPrefix()
 	{
-		$this->router->shouldReceive('dispatch')->andReturn(new Dingo\Api\Http\Response('test', 200));
+		$this->router->api(['version' => 'v1', 'prefix' => 'baz'], function()
+		{
+			$this->router->get('test', function(){ return 'test'; });
+		});
+
+		$this->auth->shouldReceive('setUser')->once()->with(null);
+
+		$this->assertEquals('test', $this->dispatcher->get('test'));
+	}
+
 
-		$this->router->shouldReceive('getDefaultApiVersion')->once()->andReturn('v1');
-		$this->router->shouldReceive('getApiVendor')->once()->andReturn('testing');
-		$this->router->shouldReceive('getApiCollection')->once()->with('v1')->andReturn(new Dingo\Api\Routing\ApiCollection('v1', ['domain' => 'testing']));
-		$this->router->shouldReceive('enableApiRouting')->once();
+	public function testInternalRequestWithDomain()
+	{
+		$this->router->api(['version' => 'v1', 'domain' => 'foo.bar'], function()
+		{
+			$this->router->get('test', function(){ return 'test'; });
+		});
 
 		$this->auth->shouldReceive('setUser')->once()->with(null);
 
@@ -74,17 +87,50 @@ public function testInternalRequestWithPrefixAndDomain()
 	 */
 	public function testInternalRequestThrowsException()
 	{
-		$this->router->shouldReceive('dispatch')->andReturn(new Dingo\Api\Http\Response('test', 500));
+		$this->router->api(['version' => 'v1'], function() {});
+
+		$this->auth->shouldReceive('setUser')->once()->with(null);
+
+		$this->dispatcher->get('test');
+	}
 
-		$this->router->shouldReceive('getDefaultApiVersion')->once()->andReturn('v1');
-		$this->router->shouldReceive('getApiVendor')->once()->andReturn('testing');
-		$this->router->shouldReceive('getApiCollection')->once()->with('v1')->andReturn(new Dingo\Api\Routing\ApiCollection('v1', ['domain' => 'testing']));
-		$this->router->shouldReceive('enableApiRouting')->once();
+
+	/**
+	 * @expectedException Symfony\Component\HttpKernel\Exception\HttpException
+	 */
+	public function testInternalRequestThrowsExceptionWhenResponseIsNotOkay()
+	{
+		$this->router->api(['version' => 'v1'], function()
+		{
+			$this->router->get('test', function()
+			{
+				return new Illuminate\Http\Response('test', 401);
+			});
+		});
 
 		$this->auth->shouldReceive('setUser')->once()->with(null);
 
 		$this->dispatcher->get('test');
 	}
+
+
+	/**
+	 * @expectedException \RuntimeException
+	 */
+	public function testPretendingToBeUserWithInvalidParameterThrowsException()
+	{
+		$this->dispatcher->be('foo');
+	}
+
+
+	public function testPretendingToBeUserSetsUserOnAuthentication()
+	{
+		$user = m::mock('Illuminate\Database\Eloquent\Model');
+
+		$this->auth->shouldReceive('setUser')->once()->with($user);
+
+		$this->dispatcher->be($user);
+	}
 
 
 }