[4.0.0] MVC Application and Router now must have a URI to process (ph…

…alcon#12380)
chilimatic · Jan 15, 2018 · 952e96f · 952e96f
1 parent d50f2be
commit 952e96f
Show file tree

Hide file tree

Showing 11 changed files with 38 additions and 140 deletions.
diff --git a/CHANGELOG-4.0.md b/CHANGELOG-4.0.md
@@ -0,0 +1,2 @@
+# [4.0.0](https://github.com/phalcon/cphalcon/releases/tag/v4.0.0) (2017-XX-XX)
+- MVC Application and Router now must have a URI to process [#12380](https://github.com/phalcon/cphalcon/pull/12380)
diff --git a/phalcon/mvc/application.zep b/phalcon/mvc/application.zep
@@ -115,7 +115,7 @@ class Application extends BaseApplication
 	/**
 	 * Handles a MVC request
 	 */
-	public function handle(string uri = null) -> <ResponseInterface> | boolean
+	public function handle(string! uri) -> <ResponseInterface> | boolean
 	{
 		var dependencyInjector, eventsManager, router, dispatcher, response, view,
 			module, moduleObject, moduleName, className, path,

diff --git a/phalcon/mvc/micro.zep b/phalcon/mvc/micro.zep
@@ -47,7 +47,7 @@ use Phalcon\Mvc\Micro\CollectionInterface;
  *     }
  * );
  *
- * $app->handle();
+ * $app->handle("/say/welcome/Phalcon");
  *</code>
  */
 class Micro extends Injectable implements \ArrayAccess
@@ -579,7 +579,7 @@ class Micro extends Injectable implements \ArrayAccess
 	 * @param string uri
 	 * @return mixed
 	 */
-	public function handle(var uri = null)
+	public function handle(string! uri)
 	{
 		var dependencyInjector, eventsManager, status = null, router, matchedRoute,
 			handler, beforeHandlers, params, returnedValue, e, errorHandler,

diff --git a/phalcon/mvc/router.zep b/phalcon/mvc/router.zep
@@ -50,7 +50,9 @@ use Phalcon\Events\EventsAwareInterface;
  *     ]
  * );
  *
- * $router->handle();
+ * $router->handle(
+ *     "/documentation/1/examples.html"
+ * );
  *
  * echo $router->getControllerName();
  * </code>
@@ -95,10 +97,6 @@ class Router implements InjectionAwareInterface, RouterInterface, EventsAwareInt
 
 	protected _notFoundPaths;
 
-	const URI_SOURCE_GET_URL = 0;
-
-	const URI_SOURCE_SERVER_REQUEST_URI = 1;
-
 	const POSITION_FIRST = 0;
 
 	const POSITION_LAST = 1;
@@ -161,52 +159,6 @@ class Router implements InjectionAwareInterface, RouterInterface, EventsAwareInt
 		return this->_eventsManager;
 	}
 
-	/**
-	 * Get rewrite info. This info is read from $_GET["_url"]. This returns '/' if the rewrite information cannot be read
-	 */
-	public function getRewriteUri() -> string
-	{
-		var url, urlParts, realUri;
-
-		/**
-		 * By default we use $_GET["url"] to obtain the rewrite information
-		 */
-		if !this->_uriSource {
-			if fetch url, _GET["_url"] {
-				if !empty url {
-					return url;
-				}
-			}
-		} else {
-			/**
-			 * Otherwise use the standard $_SERVER["REQUEST_URI"]
-			 */
-			if fetch url, _SERVER["REQUEST_URI"] {
-				let urlParts = explode("?", url),
-					realUri = urlParts[0];
-				if !empty realUri {
-					return realUri;
-				}
-			}
-		}
-		return "/";
-	}
-
-	/**
-	 * Sets the URI source. One of the URI_SOURCE_* constants
-	 *
-	 * <code>
-	 * $router->setUriSource(
-	 *     Router::URI_SOURCE_SERVER_REQUEST_URI
-	 * );
-	 * </code>
-	 */
-	public function setUriSource(var uriSource) -> <RouterInterface>
-	{
-		let this->_uriSource = uriSource;
-		return this;
-	}
-
 	/**
 	 * Set whether router must remove the extra slashes in the handled routes
 	 */
@@ -315,38 +267,26 @@ class Router implements InjectionAwareInterface, RouterInterface, EventsAwareInt
 	 * Handles routing information received from the rewrite engine
 	 *
 	 *<code>
-	 * // Read the info from the rewrite engine
-	 * $router->handle();
-	 *
-	 * // Manually passing an URL
+	 * // Passing a URL
 	 * $router->handle("/posts/edit/1");
 	 *</code>
 	 */
-	public function handle(string uri = null)
+	public function handle(string! uri)
 	{
-		var realUri, request, currentHostName, routeFound, parts,
+		var request, currentHostName, routeFound, parts,
 			params, matches, notFoundPaths,
 			vnamespace, module,  controller, action, paramsStr, strParams,
 			route, methods, dependencyInjector,
 			hostname, regexHostName, matched, pattern, handledUri, beforeMatch,
 			paths, converters, part, position, matchPosition, converter, eventsManager;
 
-		if !uri {
-			/**
-			 * If 'uri' isn't passed as parameter it reads _GET["_url"]
-			 */
-			let realUri = this->getRewriteUri();
-		} else {
-			let realUri = uri;
-		}
-
 		/**
 		 * Remove extra slashes in the route
 		 */
-		if this->_removeExtraSlashes && realUri != "/" {
-			let handledUri = rtrim(realUri, "/");
+		if this->_removeExtraSlashes && uri != "/" {
+			let handledUri = rtrim(uri, "/");
 		} else {
-			let handledUri = realUri;
+			let handledUri = uri;
 		}
 
 		let request = null,

diff --git a/phalcon/mvc/router/annotations.zep b/phalcon/mvc/router/annotations.zep
@@ -82,23 +82,14 @@ class Annotations extends Router
 	/**
 	 * Produce the routing parameters from the rewrite information
 	 */
-	public function handle(string! uri = null)
+	public function handle(string! uri)
 	{
-		var realUri, annotationsService, handlers, controllerSuffix,
+		var annotationsService, handlers, controllerSuffix,
 			scope, prefix, dependencyInjector, handler, controllerName,
 			lowerControllerName, namespaceName, moduleName, sufixed, handlerAnnotations,
 			classAnnotations, annotations, annotation, methodAnnotations, method,
 			collection;
 
-		if !uri {
-			/**
-			 * If 'uri' isn't passed as parameter it reads $_GET["_url"]
-			 */
-			let realUri = this->getRewriteUri();
-		} else {
-			let realUri = uri;
-		}
-
 		let dependencyInjector = <DiInterface> this->_dependencyInjector;
 		if typeof dependencyInjector != "object" {
 			throw new Exception("A dependency injection container is required to access the 'annotations' service");
@@ -121,7 +112,7 @@ class Annotations extends Router
 			 */
 			let prefix = scope[0];
 
-			if !empty prefix && !starts_with(realUri, prefix) {
+			if !empty prefix && !starts_with(uri, prefix) {
 				continue;
 			}
 
@@ -202,7 +193,7 @@ class Annotations extends Router
 		/**
 		 * Call the parent handle method()
 		 */
-		parent::handle(realUri);
+		parent::handle(uri);
 	}
 
 	/**

diff --git a/phalcon/mvc/routerinterface.zep b/phalcon/mvc/routerinterface.zep
@@ -53,7 +53,7 @@ interface RouterInterface
 	/**
 	 * Handles routing information received from the rewrite engine
 	 */
-	public function handle(string uri = null) -> void;
+	public function handle(string! uri) -> void;
 
 	/**
 	 * Adds a route to the router on any HTTP method

diff --git a/tests/integration/Mvc/ApplicationCest.php b/tests/integration/Mvc/ApplicationCest.php
@@ -43,16 +43,16 @@ public function singleModule(IntegrationTester $I)
         $application = new Application();
         $application->setDI($di);
 
-        $_GET['_url'] = '/test2';
-        $I->assertEquals('<html>We are here</html>' . PHP_EOL, $application->handle()->getContent());
+        $response = $application->handle("/test2");
+
+        $I->assertEquals('<html>We are here</html>' . PHP_EOL, $response->getContent());
     }
 
     public function modulesDefinition(IntegrationTester $I)
     {
         $I->wantTo('handle request and get content by using single modules strategy (standard definition)');
 
         Di::reset();
-        $_GET['_url'] = '/index';
 
         $di = new FactoryDefault();
         $di->set('router', function () {
@@ -81,15 +81,17 @@ public function modulesDefinition(IntegrationTester $I)
         ]);
 
         $application->setDI($di);
-        $I->assertEquals('<html>here</html>' . PHP_EOL, $application->handle()->getContent());
+
+        $response = $application->handle("/index");
+
+        $I->assertEquals('<html>here</html>' . PHP_EOL, $response->getContent());
     }
 
     public function modulesClosure(IntegrationTester $I)
     {
         $I->wantTo('handle request and get content by using single modules strategy (closure)');
 
         Di::reset();
-        $_GET['_url'] = '/login';
 
         $di = new FactoryDefault();
         $di->set('router', function () {
@@ -131,6 +133,9 @@ public function modulesClosure(IntegrationTester $I)
         ]);
 
         $application->setDI($di);
-        $I->assertEquals('<html>here</html>' . PHP_EOL, $application->handle()->getContent());
+
+        $response = $application->handle("/login");
+
+        $I->assertEquals('<html>here</html>' . PHP_EOL, $response->getContent());
     }
 }
diff --git a/tests/integration/Mvc/Dispatcher/ForwardCest.php b/tests/integration/Mvc/Dispatcher/ForwardCest.php
@@ -62,7 +62,7 @@ public function handlingException(IntegrationTester $I)
         $application->setEventsManager(new Manager());
         $application->setDI($di);
 
-        $_GET['_url'] = '/exception';
-        $I->assertSame("I should be displayed", $application->handle()->getContent());
+        $response = $application->handle("/exception");
+        $I->assertSame("I should be displayed", $response->getContent());
     }
 }
diff --git a/tests/unit/Mvc/MicroTest.php b/tests/unit/Mvc/MicroTest.php
@@ -185,25 +185,22 @@ function () {
 
                 //Getting the url from _url using GET
                 $_SERVER["REQUEST_METHOD"] = "GET";
-                $_GET["_url"] = "/api/site";
 
-                $app->handle();
+                $app->handle("/api/site");
 
                 expect($handler->getNumberAccess())->equals(1);
                 expect($handler->getTrace())->equals(["find"]);
 
                 //Getting the url from _url using POST
                 $_SERVER["REQUEST_METHOD"] = "POST";
-                $_GET["_url"] = "/api/site/save";
 
-                $app->handle();
+                $app->handle("/api/site/save");
 
                 expect($handler->getNumberAccess())->equals(2);
                 expect($handler->getTrace())->equals(["find", "save"]);
 
                 //Passing directly a URI
                 $_SERVER["REQUEST_METHOD"] = "DELETE";
-                $_GET["_url"] = null;
 
                 $app->handle("/api/site/delete/1");
 
@@ -241,9 +238,8 @@ function () use (&$flag) {
                 );
 
                 $_SERVER["REQUEST_METHOD"] = "GET";
-                $_GET["_url"] = "/fourohfour";
 
-                $app->handle();
+                $app->handle("/fourohfour");
 
                 expect($flag)->true();
             }

diff --git a/tests/unit/Mvc/Router/AnnotationsTest.php b/tests/unit/Mvc/Router/AnnotationsTest.php
@@ -98,7 +98,7 @@ function ($uri, $method, $controller, $action, $params) {
                 $router->addResource("About");
                 $router->addResource("Main");
 
-                $router->handle();
+                $router->handle("/");
 
                 expect($router->getRoutes())->count(9);
 

diff --git a/tests/unit/Mvc/RouterTest.php b/tests/unit/Mvc/RouterTest.php
@@ -110,7 +110,7 @@ function () {
                         return true;
                     });
 
-                $router->handle();
+                $router->handle("/");
                 expect($router->wasMatched())->false();
 
                 $router->handle('/static/route');
@@ -526,7 +526,7 @@ function () {
                     ]
                 );
 
-                $router->handle();
+                $router->handle("/");
 
                 expect($router->getControllerName())->equals('controller');
                 expect($router->getActionName())->equals('action');
@@ -536,36 +536,6 @@ function () {
         );
     }
 
-    /**
-     * Tests setting different URI source
-     *
-     * @author Andy Gutierrez <[email protected]>
-     * @since  2013-04-07
-     */
-    public function testMatchingByUsingDifferentUriSource()
-    {
-        $this->specify(
-            'Matching uri when setting different uri source does not work as expected',
-            function () {
-                $router = $this->getRouter(false);
-
-                $_GET['_url'] = '/some/route';
-                expect($router->getRewriteUri())->equals('/some/route');
-
-                $router->setUriSource(Router::URI_SOURCE_GET_URL);
-                expect($router->getRewriteUri())->equals('/some/route');
-
-                $_SERVER['REQUEST_URI'] = '/some/route';
-                $router->setUriSource(Router::URI_SOURCE_SERVER_REQUEST_URI);
-
-                expect($router->getRewriteUri())->equals('/some/route');
-
-                $_SERVER['REQUEST_URI'] = '/some/route?x=1';
-                expect($router->getRewriteUri())->equals('/some/route');
-            }
-        );
-    }
-
     protected function convertersProvider()
     {
         return [
@@ -818,12 +788,6 @@ protected function methodProvider()
     protected function routerProvider()
     {
         return [
-            [
-                'uri' => '',
-                'controller' => 'index',
-                'action' => 'index',
-                'params' => []
-            ],
             [
                 'uri' => '/',
                 'controller' => 'index',
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		# [4.0.0](https://github.com/phalcon/cphalcon/releases/tag/v4.0.0) (2017-XX-XX)
		- MVC Application and Router now must have a URI to process [#12380](https://github.com/phalcon/cphalcon/pull/12380)