Unify App::url() code, convert string page to array early (#2108)

atk4 · Sep 15, 2023 · b385284 · b385284
1 parent f1277b7
commit b385284
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 26 deletions.
diff --git a/src/App.php b/src/App.php
@@ -612,10 +612,10 @@ protected function createRequestPathFromLocalPath(string $localPath): string
             if (\PHP_SAPI === 'cli') { // for phpunit
                 $requestUrlPath = '/';
                 $requestLocalPath = \Closure::bind(static function () {
-                    return dirname((new ExceptionRenderer\Html(new \Exception()))->getVendorDirectory());
+                    return (new ExceptionRenderer\Html(new \Exception()))->getVendorDirectory();
                 }, null, ExceptionRenderer\Html::class)();
             } else {
-                $request = \Symfony\Component\HttpFoundation\Request::createFromGlobals();
+                $request = new \Symfony\Component\HttpFoundation\Request([], [], [], [], [], $_SERVER);
                 $requestUrlPath = $request->getBasePath();
                 $requestLocalPath = realpath($request->server->get('SCRIPT_FILENAME'));
             }
@@ -657,37 +657,39 @@ public function stickyForget(string $name): void
      */
     public function url($page = [], array $extraRequestUrlArgs = []): string
     {
-        $request = $this->getRequest();
-
-        $pagePath = '';
         if (is_string($page)) {
             $pageExploded = explode('?', $page, 2);
-            $pagePath = $pageExploded[0];
             parse_str($pageExploded[1] ?? '', $page);
+            $pagePath = $pageExploded[0] !== '' ? $pageExploded[0] : null;
         } else {
-            if (isset($page[0])) {
-                $pagePath = $page[0];
-            } else {
-                // use current page by default
-                $requestUrl = $request->getUri()->getPath();
-                if (substr($requestUrl, -1, 1) === '/') {
-                    $pagePath = $this->urlBuildingIndexPage;
-                } else {
-                    $pagePath = basename($requestUrl, $this->urlBuildingExt);
-                }
-            }
+            $pagePath = $page[0] ?? null;
             unset($page[0]);
-            if ($pagePath) {
-                $pagePath .= $this->urlBuildingExt;
+        }
+
+        $request = $this->getRequest();
+
+        if ($pagePath === null) {
+            $pagePath = $request->getUri()->getPath();
+            if ($pagePath === '') { // TODO path must always start with '/'
+                $pagePath = '/';
             }
+            if (substr($pagePath, -1) === '/') {
+                $pagePath = $this->urlBuildingIndexPage;
+            } else {
+                $pagePath = basename($pagePath, $this->urlBuildingExt);
+            }
+        }
+        if (!str_contains(basename($pagePath), '.')) {
+            $pagePath .= $this->urlBuildingExt;
         }
 
         $args = $extraRequestUrlArgs;
 
         // add sticky arguments
+        $requestQueryParams = $request->getQueryParams();
         foreach ($this->stickyGetArguments as $k => $v) {
-            if ($v && isset($_GET[$k])) {
-                $args[$k] = $_GET[$k];
+            if ($v && isset($requestQueryParams[$k])) {
+                $args[$k] = $requestQueryParams[$k];
             } else {
                 unset($args[$k]);
             }
@@ -702,11 +704,9 @@ public function url($page = [], array $extraRequestUrlArgs = []): string
             }
         }
 
-        // put URL together
         $pageQuery = http_build_query($args, '', '&', \PHP_QUERY_RFC3986);
-        $url = $pagePath . ($pageQuery ? '?' . $pageQuery : '');
 
-        return $url;
+        return $pagePath . ($pageQuery !== '' ? '?' . $pageQuery : '');
     }
 
     /**

diff --git a/tests/CallbackTest.php b/tests/CallbackTest.php
@@ -89,8 +89,8 @@ public function testViewUrlCallback(): void
         $this->simulateCallbackTriggering($cbApp);
         $this->simulateCallbackTriggering($cb);
 
-        $expectedUrlCbApp = '?' . Callback::URL_QUERY_TRIGGER_PREFIX . 'aa=callback&' . Callback::URL_QUERY_TARGET . '=aa';
-        $expectedUrlCb = '?' . /* Callback::URL_QUERY_TRIGGER_PREFIX . 'aa=1&' . */ Callback::URL_QUERY_TRIGGER_PREFIX . 'bb=callback&' . Callback::URL_QUERY_TARGET . '=bb';
+        $expectedUrlCbApp = 'index.php?' . Callback::URL_QUERY_TRIGGER_PREFIX . 'aa=callback&' . Callback::URL_QUERY_TARGET . '=aa';
+        $expectedUrlCb = 'index.php?' . /* Callback::URL_QUERY_TRIGGER_PREFIX . 'aa=1&' . */ Callback::URL_QUERY_TRIGGER_PREFIX . 'bb=callback&' . Callback::URL_QUERY_TARGET . '=bb';
         self::assertSame($expectedUrlCbApp, $cbApp->getUrl());
         self::assertSame($expectedUrlCb, $cb->getUrl());