Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 10 commits into from
Sep 15, 2023
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
342bfb4
fix URL for non properly configured $_SERVER
mvorisek Sep 15, 2023
008e43f
merge URL params from request API (instead of $_GET)
mvorisek Sep 14, 2023
8fadf94
stricter URL query concat
mvorisek Sep 14, 2023
d4afbbd
convert page string to array and then process the same way
mvorisek Sep 14, 2023
df0c7d4
extract $page[0] into $pagePage early
mvorisek Sep 14, 2023
28c3f3e
rm dirname in App::createRequestPathFromLocalPath() for phpunit
mvorisek Sep 14, 2023
b492b5e
unwrap Symfony call to stress $_GET/$_SERVER global vars
mvorisek Sep 14, 2023
9f67fd0
$_GET in App::createRequestPathFromLocalPath() is not needed/used
mvorisek Sep 14, 2023
ed8fac4
refactor for cleaness
mvorisek Sep 15, 2023
6c902dd
do not add ext if basename part contains dot
mvorisek Sep 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 24 additions & 24 deletions src/App.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
}
Expand Down Expand Up @@ -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]);
}
Expand All @@ -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 : '');
}

/**
Expand Down
4 changes: 2 additions & 2 deletions tests/CallbackTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand Down