From 80b5ac07d0cea2dc685d3e871c60dd94a84fb110 Mon Sep 17 00:00:00 2001 From: Roman Date: Fri, 29 Mar 2024 13:33:29 +0100 Subject: [PATCH] Router bugfix, save and update route frontend --- Core/API/ApiKeyAPI.class.php | 7 +- Core/API/RoutesAPI.class.php | 133 ++++----------- Core/Objects/DatabaseEntity/Route.class.php | 11 +- .../Router/RedirectPermanentlyRoute.class.php | 12 +- Core/Objects/Router/RedirectRoute.class.php | 2 +- .../Router/RedirectTemporaryRoute.class.php | 12 +- Core/Objects/Router/StaticRoute.class.php | 2 +- react/admin-panel/src/AdminDashboard.jsx | 4 +- .../src/views/access-control-list.js | 9 +- .../admin-panel/src/views/route/route-edit.js | 153 ++++++++++++++++++ .../admin-panel/src/views/route/route-form.js | 105 ++++++++++++ .../views/{routes.js => route/route-list.js} | 18 +-- react/shared/api.js | 20 ++- 13 files changed, 352 insertions(+), 136 deletions(-) create mode 100644 react/admin-panel/src/views/route/route-edit.js create mode 100644 react/admin-panel/src/views/route/route-form.js rename react/admin-panel/src/views/{routes.js => route/route-list.js} (95%) diff --git a/Core/API/ApiKeyAPI.class.php b/Core/API/ApiKeyAPI.class.php index c87f98b..0dcde96 100644 --- a/Core/API/ApiKeyAPI.class.php +++ b/Core/API/ApiKeyAPI.class.php @@ -9,6 +9,7 @@ namespace Core\API { public function __construct(Context $context, bool $externalCall = false, array $params = array()) { parent::__construct($context, $externalCall, $params); + $this->loginRequired = true; } protected function fetchAPIKey(int $apiKeyId): ApiKey|bool { @@ -41,7 +42,6 @@ namespace Core\API\ApiKey { public function __construct(Context $context, $externalCall = false) { parent::__construct($context, $externalCall, array()); $this->apiKeyAllowed = false; - $this->loginRequired = true; } public function _execute(): bool { @@ -73,9 +73,7 @@ namespace Core\API\ApiKey { public function __construct(Context $context, $externalCall = false) { $params = $this->getPaginationParameters(["token", "validUntil", "active"]); $params["showActiveOnly"] = new Parameter("showActiveOnly", Parameter::TYPE_BOOLEAN, true, true); - parent::__construct($context, $externalCall, $params); - $this->loginRequired = true; } public function _execute(): bool { @@ -119,7 +117,7 @@ namespace Core\API\ApiKey { parent::__construct($context, $externalCall, array( "id" => new Parameter("id", Parameter::TYPE_INT), )); - $this->loginRequired = true; + $this->apiKeyAllowed = false; } public function _execute(): bool { @@ -147,7 +145,6 @@ namespace Core\API\ApiKey { parent::__construct($user, $externalCall, array( "id" => new Parameter("id", Parameter::TYPE_INT), )); - $this->loginRequired = true; } public function _execute(): bool { diff --git a/Core/API/RoutesAPI.class.php b/Core/API/RoutesAPI.class.php index 8f6cb60..94282fa 100644 --- a/Core/API/RoutesAPI.class.php +++ b/Core/API/RoutesAPI.class.php @@ -76,6 +76,7 @@ namespace Core\API\Routes { use Core\Objects\DatabaseEntity\Route; use Core\Objects\Router\ApiRoute; use Core\Objects\Router\Router; + use Core\Objects\Router\StaticRoute; class Fetch extends RoutesAPI { @@ -102,108 +103,40 @@ namespace Core\API\Routes { } } - class Save extends RoutesAPI { - - private array $routes; + class Get extends RoutesAPI { public function __construct(Context $context, $externalCall = false) { - parent::__construct($context, $externalCall, array( - 'routes' => new Parameter('routes', Parameter::TYPE_ARRAY, false) - )); + parent::__construct($context, $externalCall, [ + "id" => new Parameter("id", Parameter::TYPE_INT) + ]); } public function _execute(): bool { - if (!$this->validateRoutes()) { - return false; - } - $sql = $this->context->getSQL(); - $sql->startTransaction(); + $routeId = $this->getParam("id"); - // DELETE old rules; - $this->success = ($sql->truncate("Route")->execute() !== FALSE); + $route = Route::find($sql, $routeId); $this->lastError = $sql->getLastError(); - - // INSERT new routes - if ($this->success) { - $insertStatement = Route::getHandler($sql)->getInsertQuery($this->routes); - $this->success = ($insertStatement->execute() !== FALSE); - $this->lastError = $sql->getLastError(); - } + $this->success = ($route !== FALSE); if ($this->success) { - $sql->commit(); - return $this->regenerateCache(); - } else { - $sql->rollback(); - return false; - } - } - - private function validateRoutes(): bool { - - $this->routes = array(); - $keys = array( - "id" => Parameter::TYPE_INT, - "pattern" => [Parameter::TYPE_STRING, Parameter::TYPE_INT], - "type" => Parameter::TYPE_STRING, - "target" => Parameter::TYPE_STRING, - "extra" => Parameter::TYPE_STRING, - "active" => Parameter::TYPE_BOOLEAN, - "exact" => Parameter::TYPE_BOOLEAN, - ); - - foreach ($this->getParam("routes") as $index => $route) { - foreach ($keys as $key => $expectedType) { - if (!array_key_exists($key, $route)) { - if ($key !== "id") { // id is optional - return $this->createError("Route $index missing key: $key"); - } else { - continue; - } - } - - $value = $route[$key]; - $type = Parameter::parseType($value); - if (!is_array($expectedType)) { - $expectedType = [$expectedType]; - } - - if (!in_array($type, $expectedType)) { - if (count($expectedType) > 0) { - $expectedTypeName = "expected: " . Parameter::names[$expectedType]; - } else { - $expectedTypeName = "expected one of: " . implode(",", array_map( - function ($type) { - return Parameter::names[$type]; - }, $expectedType)); - } - $gotTypeName = Parameter::names[$type]; - return $this->createError("Route $index has invalid value for key: $key, $expectedTypeName, got: $gotTypeName"); - } + if ($route === null) { + return $this->createError("Route not found"); + } else { + $this->result["route"] = $route; } - - $type = $route["type"]; - if (!isset(Route::ROUTE_TYPES[$type])) { - return $this->createError("Invalid type: $type"); - } - - if (empty($route["pattern"])) { - return $this->createError("Pattern cannot be empty."); - } - - if (empty($route["target"])) { - return $this->createError("Target cannot be empty."); - } - - $this->routes[] = $route; } - return true; + return $this->success; } public static function getDefaultACL(Insert $insert): void { - $insert->addRow(self::getEndpoint(), [Group::ADMIN], "Allows users to save the site routing", true); + $insert->addRow( + self::getEndpoint(), + [Group::ADMIN, Group::MODERATOR], + "Allows users to fetch a single route", + true + ); } } @@ -218,7 +151,6 @@ namespace Core\API\Routes { "exact" => new Parameter("exact", Parameter::TYPE_BOOLEAN), "active" => new Parameter("active", Parameter::TYPE_BOOLEAN, true, true), )); - $this->isPublic = false; } public function _execute(): bool { @@ -237,6 +169,11 @@ namespace Core\API\Routes { $sql = $this->context->getSQL(); $this->success = $route->save($sql) !== false; $this->lastError = $sql->getLastError(); + + if ($this->success) { + $this->result["routeId"] = $route->getId(); + } + return $this->success && $this->regenerateCache(); } @@ -256,7 +193,6 @@ namespace Core\API\Routes { "exact" => new Parameter("exact", Parameter::TYPE_BOOLEAN), "active" => new Parameter("active", Parameter::TYPE_BOOLEAN, true, true), )); - $this->isPublic = false; } public function _execute(): bool { @@ -417,24 +353,17 @@ namespace Core\API\Routes { class Check extends RoutesAPI { public function __construct(Context $context, bool $externalCall) { parent::__construct($context, $externalCall, [ - "id" => new Parameter("id", Parameter::TYPE_INT), - "path" => new StringType("path") + "pattern" => new StringType("pattern", 128), + "path" => new StringType("path"), + "exact" => new Parameter("exact", Parameter::TYPE_BOOLEAN, true, true) ]); } protected function _execute(): bool { - $sql = $this->context->getSQL(); - $routeId = $this->getParam("id"); - $route = Route::find($sql, $routeId); - if ($route === false) { - $this->lastError = $sql->getLastError(); - return false; - } else if ($route === null) { - return $this->createError("Route not found"); - } - - $this->success = true; $path = $this->getParam("path"); + $pattern = $this->getParam("pattern"); + $exact = $this->getParam("exact"); + $route = new StaticRoute($pattern, $exact, ""); $this->result["match"] = $route->match($path); return $this->success; } @@ -442,7 +371,7 @@ namespace Core\API\Routes { public static function getDefaultACL(Insert $insert): void { $insert->addRow("routes/check", [Group::ADMIN, Group::MODERATOR], - "Users with this permission can see, if a route is matched with the given path for debugging purposes", + "Users with this permission can see, if a route pattern is matched with the given path for debugging purposes", true ); } diff --git a/Core/Objects/DatabaseEntity/Route.class.php b/Core/Objects/DatabaseEntity/Route.class.php index 45cb640..634f767 100644 --- a/Core/Objects/DatabaseEntity/Route.class.php +++ b/Core/Objects/DatabaseEntity/Route.class.php @@ -10,7 +10,8 @@ use Core\Objects\DatabaseEntity\Attribute\MaxLength; use Core\Objects\DatabaseEntity\Attribute\Unique; use Core\Objects\DatabaseEntity\Controller\DatabaseEntity; use Core\Objects\Router\DocumentRoute; -use Core\Objects\Router\RedirectRoute; +use Core\Objects\Router\RedirectPermanentlyRoute; +use Core\Objects\Router\RedirectTemporaryRoute; use Core\Objects\Router\Router; use Core\Objects\Router\StaticFileRoute; @@ -23,8 +24,8 @@ abstract class Route extends DatabaseEntity { const TYPE_REDIRECT_PERMANENTLY = "redirect_permanently"; const TYPE_REDIRECT_TEMPORARY = "redirect_temporary"; const ROUTE_TYPES = [ - self::TYPE_REDIRECT_TEMPORARY => RedirectRoute::class, - self::TYPE_REDIRECT_PERMANENTLY => RedirectRoute::class, + self::TYPE_REDIRECT_TEMPORARY => RedirectTemporaryRoute::class, + self::TYPE_REDIRECT_PERMANENTLY => RedirectPermanentlyRoute::class, self::TYPE_STATIC => StaticFileRoute::class, self::TYPE_DYNAMIC => DocumentRoute::class ]; @@ -60,6 +61,10 @@ abstract class Route extends DatabaseEntity { return $this->active; } + public function isExact(): bool { + return $this->exact; + } + private static function parseParamType(?string $type): ?int { if ($type === null || trim($type) === "") { return null; diff --git a/Core/Objects/Router/RedirectPermanentlyRoute.class.php b/Core/Objects/Router/RedirectPermanentlyRoute.class.php index f8cafe1..f30555f 100644 --- a/Core/Objects/Router/RedirectPermanentlyRoute.class.php +++ b/Core/Objects/Router/RedirectPermanentlyRoute.class.php @@ -2,8 +2,18 @@ namespace Core\Objects\Router; +use Core\Driver\SQL\SQL; + class RedirectPermanentlyRoute extends RedirectRoute { + + const HTTP_STATUS_CODE = 308; + public function __construct(string $pattern, bool $exact, string $destination) { - parent::__construct("redirect_permanently", $pattern, $exact, $destination, 308); + parent::__construct("redirect_permanently", $pattern, $exact, $destination, self::HTTP_STATUS_CODE); + } + + public function postFetch(SQL $sql, array $row) { + parent::postFetch($sql, $row); + $this->code = self::HTTP_STATUS_CODE; } } \ No newline at end of file diff --git a/Core/Objects/Router/RedirectRoute.class.php b/Core/Objects/Router/RedirectRoute.class.php index 1c0ea39..60d6ebd 100644 --- a/Core/Objects/Router/RedirectRoute.class.php +++ b/Core/Objects/Router/RedirectRoute.class.php @@ -9,7 +9,7 @@ use JetBrains\PhpStorm\Pure; class RedirectRoute extends Route { #[Transient] - private int $code; + protected int $code; public function __construct(string $type, string $pattern, bool $exact, string $destination, int $code = 307) { parent::__construct($type, $pattern, $destination, $exact); diff --git a/Core/Objects/Router/RedirectTemporaryRoute.class.php b/Core/Objects/Router/RedirectTemporaryRoute.class.php index e77745a..a9c15ac 100644 --- a/Core/Objects/Router/RedirectTemporaryRoute.class.php +++ b/Core/Objects/Router/RedirectTemporaryRoute.class.php @@ -2,8 +2,18 @@ namespace Core\Objects\Router; +use Core\Driver\SQL\SQL; + class RedirectTemporaryRoute extends RedirectRoute { + + const HTTP_STATUS_CODE = 307; + public function __construct(string $pattern, bool $exact, string $destination) { - parent::__construct("redirect_temporary", $pattern, $exact, $destination, 307); + parent::__construct("redirect_temporary", $pattern, $exact, $destination, self::HTTP_STATUS_CODE); + } + + public function postFetch(SQL $sql, array $row) { + parent::postFetch($sql, $row); + $this->code = self::HTTP_STATUS_CODE; } } \ No newline at end of file diff --git a/Core/Objects/Router/StaticRoute.class.php b/Core/Objects/Router/StaticRoute.class.php index 553700f..81104bb 100644 --- a/Core/Objects/Router/StaticRoute.class.php +++ b/Core/Objects/Router/StaticRoute.class.php @@ -14,7 +14,7 @@ class StaticRoute extends Route { private int $code; public function __construct(string $pattern, bool $exact, string $data, int $code = 200) { - parent::__construct("static", $pattern, $exact); + parent::__construct("static", $pattern, "", $exact); $this->data = $data; $this->code = $code; } diff --git a/react/admin-panel/src/AdminDashboard.jsx b/react/admin-panel/src/AdminDashboard.jsx index 1e0e14b..b964fe3 100644 --- a/react/admin-panel/src/AdminDashboard.jsx +++ b/react/admin-panel/src/AdminDashboard.jsx @@ -20,7 +20,8 @@ const GroupListView = lazy(() => import('./views/group/group-list')); const EditGroupView = lazy(() => import('./views/group/group-edit')); const LogView = lazy(() => import("./views/log-view")); const AccessControlList = lazy(() => import("./views/access-control-list")); -const RouteListView = lazy(() => import("./views/routes")); +const RouteListView = lazy(() => import("./views/route/route-list")); +const RouteEditView = lazy(() => import("./views/route/route-edit")); export default function AdminDashboard(props) { @@ -81,6 +82,7 @@ export default function AdminDashboard(props) { }/> }/> }/> + }/> } /> diff --git a/react/admin-panel/src/views/access-control-list.js b/react/admin-panel/src/views/access-control-list.js index 8b5aecc..4a74b6e 100644 --- a/react/admin-panel/src/views/access-control-list.js +++ b/react/admin-panel/src/views/access-control-list.js @@ -18,6 +18,10 @@ import {Add, Delete, Edit, Refresh} from "@material-ui/icons"; import {USER_GROUP_ADMIN} from "shared/constants"; import Dialog from "shared/elements/dialog"; +const BorderedColumn = styled(TableCell)({ + borderLeft: "1px dotted #666", + borderRight: "1px dotted #666", +}); export default function AccessControlList(props) { @@ -203,11 +207,6 @@ export default function AccessControlList(props) { return <>{rows} } - const BorderedColumn = styled(TableCell)({ - borderLeft: "1px dotted #666", - borderRight: "1px dotted #666", - }); - return <>
diff --git a/react/admin-panel/src/views/route/route-edit.js b/react/admin-panel/src/views/route/route-edit.js new file mode 100644 index 0000000..df94b88 --- /dev/null +++ b/react/admin-panel/src/views/route/route-edit.js @@ -0,0 +1,153 @@ +import {Link, useNavigate, useParams} from "react-router-dom"; +import {useCallback, useContext, useEffect, useState} from "react"; +import {LocaleContext} from "shared/locale"; +import { + Box, + Button, + CircularProgress, styled, +} from "@material-ui/core"; +import * as React from "react"; +import RouteForm from "./route-form"; +import {KeyboardArrowLeft, Save} from "@material-ui/icons"; +import {TextField} from "@mui/material"; + +const ButtonBar = styled(Box)((props) => ({ + "& > button": { + marginRight: props.theme.spacing(1) + } +})); + +export default function RouteEditView(props) { + + const {api, showDialog} = props; + const {routeId} = useParams(); + const navigate = useNavigate(); + const isNewRoute = routeId === "new"; + const {translate: L, requestModules, currentLocale} = useContext(LocaleContext); + + // data + const [routeTest, setRouteTest] = useState(""); + const [fetchRoute, setFetchRoute] = useState(!isNewRoute); + const [route, setRoute] = useState(isNewRoute ? { + pattern: "", + type: "", + target: "", + extra: "", + exact: true, + active: true + } : null); + + // ui + const [routeTestResult, setRouteTestResult] = useState(false); + const [isSaving, setSaving] = useState(false); + + useEffect(() => { + requestModules(props.api, ["general"], currentLocale).then(data => { + if (!data.success) { + props.showDialog("Error fetching translations: " + data.msg); + } + }); + }, [currentLocale]); + + useEffect(() => { + if (routeTest?.trim()) { + props.api.testRoute(route.pattern, routeTest, route.exact).then(data => { + if (!data.success) { + props.showDialog("Error testing route: " + data.msg); + } else { + setRouteTestResult(data.match); + } + }); + } else { + setRouteTestResult(false); + } + }, [routeTest]); + + const onFetchRoute = useCallback((force = false) => { + if (!isNewRoute && (force || fetchRoute)) { + setFetchRoute(false); + api.getRoute(routeId).then((res) => { + if (!res.success) { + showDialog(res.msg, L("Error fetching route")); + navigate("/admin/routes"); + } else { + setRoute(res.route); + } + }); + } + }, [api, showDialog, fetchRoute, isNewRoute, routeId, route]); + + const onSave = useCallback(() => { + if (!isSaving) { + setSaving(true); + let args = [route.pattern, route.type, route.target, route.extra, route.exact, route.active]; + if (isNewRoute) { + api.addRoute(...args).then(res => { + setSaving(false); + if (res.success) { + navigate("/admin/routes/" + res.routeId); + } else { + showDialog(res.msg, L("Error saving route")); + } + }); + } else { + args = [routeId, ...args]; + api.updateRoute(...args).then(res => { + setSaving(false); + if (!res.success) { + showDialog(res.msg, L("Error saving route")); + } + }); + } + } + }, [api, route, isSaving, isNewRoute, routeId]); + + useEffect(() => { + if (!isNewRoute) { + onFetchRoute(true); + } + }, []); + + if (route === null) { + return + } + + return
+
+
    +
  1. Home
    2. +
  2. Routes
    3. +
  3. {isNewRoute ? "New" : "Edit"}
    4. +
+
+
+
+

{L(isNewRoute ? "Create new Route" : "Edit Route")}

+
+
+
+
+ + + + + + +
{L("Validate Route")}
+ setRouteTest(e.target.value)} + variant={"outlined"} size={"small"} fullWidth={true} + placeholder={L("Enter a path to test the route…")} /> + 
+                Match: {JSON.stringify(routeTestResult)}
+
+ +
+} \ No newline at end of file diff --git a/react/admin-panel/src/views/route/route-form.js b/react/admin-panel/src/views/route/route-form.js new file mode 100644 index 0000000..5187f2d --- /dev/null +++ b/react/admin-panel/src/views/route/route-form.js @@ -0,0 +1,105 @@ +import {Box, Checkbox, FormControl, FormControlLabel, FormGroup, Select, styled, TextField} from "@material-ui/core"; +import * as React from "react"; +import {useCallback, useContext} from "react"; +import {LocaleContext} from "shared/locale"; + +const RouteFormControl = styled(FormControl)((props) => ({ + "& > label": { + marginTop: 5 + }, + "& input, & textarea": { + fontFamily: "monospace", + } +})); + +export default function RouteForm(props) { + + const {route, setRoute} = props; + const {translate: L} = useContext(LocaleContext); + + const onChangeRouteType = useCallback((type) => { + let newRoute = {...route, type: type }; + if (newRoute.type === "dynamic" && !newRoute.extra) { + newRoute.extra = "[]"; + } else if (newRoute.type === "static" && !newRoute.extra) { + newRoute.extra = 200; + } + + setRoute(newRoute); + }, [route]); + + const elements = [ + + + setRoute({...route, pattern: e.target.value})} /> + , + + setRoute({...route, exact: e.target.checked})} />} /> + , + + setRoute({...route, active: e.target.checked})} />} /> + , + + + + , + ]; + + if (route.type) { + elements.push( + + + setRoute({...route, target: e.target.value})}/> + + ); + + if (route.type === "dynamic") { + let extraArgs; + try { + extraArgs = JSON.parse(route.extra) + } catch (e) { + extraArgs = null + } + elements.push( + + +