From 1bf3dfa3b659c664dc1eef925d0818f119d26a5a Mon Sep 17 00:00:00 2001 From: Override-6 Date: Sun, 26 Nov 2023 00:09:19 +0100 Subject: [PATCH] add and fix conception --- Documentation/Conception.md | 123 ++++++++++++++++++ public/api/index.php | 26 +++- public/index.php | 10 +- src/Api/API.php | 26 +--- src/Api/Controller/APIAuthController.php | 2 +- src/Api/Controller/APITacticController.php | 2 +- src/App/App.php | 2 +- src/App/Control.php | 16 +-- src/App/Controller/AuthController.php | 5 +- src/App/Controller/EditorController.php | 10 +- src/App/Controller/TeamController.php | 4 +- src/App/Controller/UserController.php | 3 +- src/App/Controller/VisualizerController.php | 8 +- .../Session/MutableSessionHandle.php | 2 +- .../Session/PhpSessionHandle.php | 2 +- src/{Core => App}/Session/SessionHandle.php | 2 +- .../Validator/TacticValidator.php | 13 +- src/Core/Validation/ComposedValidator.php | 5 +- src/Core/Validation/Validator.php | 2 +- src/{utils.php => index-utils.php} | 0 20 files changed, 188 insertions(+), 75 deletions(-) create mode 100644 Documentation/Conception.md rename src/{Core => App}/Session/MutableSessionHandle.php (93%) rename src/{Core => App}/Session/PhpSessionHandle.php (96%) rename src/{Core => App}/Session/SessionHandle.php (94%) rename src/{Core => App}/Validator/TacticValidator.php (59%) rename src/{utils.php => index-utils.php} (100%) diff --git a/Documentation/Conception.md b/Documentation/Conception.md new file mode 100644 index 0000000..68b4cd9 --- /dev/null +++ b/Documentation/Conception.md @@ -0,0 +1,123 @@ +# Conception + +## Organisation + +Notre projet est divisé en plusieurs parties: + +- `src/API`, qui définit les classes qui implémentent les actions de l’api +- `src/App`, qui définit les contrôleurs et les vues de l’application web +- `src/Core`, définit les modèles, les classes métiers, les librairies internes (validation, http), les gateways, en somme, les élements logiques de l’application et les actions que l’ont peut faire avec. +- `sql`, définit la base de donnée utilisée, et éxécute les fichiers sql lorsque la base de donnée n’est pas initialisée. +- `profiles`, définit les profiles d’execution, voir `Documentation/how-to-dev.md` pour plus d’info +- `front` contient le code front-end react/typescript +- `ci` contient les scripts de déploiement et la définition du workflow d’intégration continue et de déploiement constant vers notre staging server ([maxou.dev//public/](https://maxou.dev/IQBall/master/public)). +- `public` point d’entrée, avec : + - `public/index.php` point d’entrée pour la webapp + - `public/api/index.php` point d’entrée pour l’api. + +## Backend + +### Validation et résilience des erreurs +#### Motivation +Un controlleur a pour but de valider les données d'une requête avant de les manipuler. + +Nous nous sommes rendu compte que la vérification des données d'une requête était redondante, même en factorisant les différents +types de validation, nous devions quand même explicitement vérifier la présence des champs utilisés dans la requête. + +```php +public function doPostAction(array $form) { + $failures = []; + $req = new HttpRequest($form); + $email = $req['email'] ?? null; + if ($email == null) { + $failures[] = "Vous n'avez pas entré d'adresse email."; + return; + } + if (Validation::isEmail($email)) { + $failures[] = "Format d'adresse email invalide."; + } + if (Validation::isLenBetween($email, 6, 64))) { + $failures[] = "L'adresse email doit être d'une longueur comprise entre 6 et 64 charactères."; + } + + if (!empty($failures)) { + return ViewHttpResponse::twig('error.html.twig', ['failures' => $failures]); + } + + // traitement ... +} +``` + +Nous sommes obligés de tester à la main la présence des champs dans la requête, et nous avons une paire condition/erreur par type de validation, +ce qui, pour des requêtes avec plusieurs champs, peut vite devenir illisible si nous voulons être précis sur les erreurs. + +Ici, une validation est une règle, un prédicat qui permet de valider une donnée sur un critère bien précis (injection html, adresse mail, longueur, etc.). +Bien souvent, lorsque le prédicat échoue, un message est ajouté à la liste des erreurs rencontrés, mais ce message est souvent le même, ce qui rajoute en plus +de la redondance sur les messages d'erreurs choisis lorsqu'une validation échoue. + +#### Schéma +Toutes ces lignes de code pour définir que notre requête doit contenir un champ nommé `email`, dont la valeur est une adresse mail, d'une longueur comprise entre 6 et 64. +Nous avons donc trouvé l'idée de définir un système nous permettant de déclarer le schéma d'une requête, +et de reporter le plus d'erreurs possibles lorsqu'une requête ne valide pas le schéma : + +```php +public function doPostAction(array $form): HttpResponse { + $failures = []; + $req = HttpRequest::from($form, $failures, [ + 'email' => [Validators::email(), Validators::isLenBetween(6, 64)] + ]); + + if (!empty($failures)) { //ou $req == null + return ViewHttpResponse::twig('error.html.twig', ['failures' => $failures]) + } + + // traitement ... +} +``` +Ce système nous permet de faire de la programmation _déclarative_, nous disons à _quoi_ ressemble la forme de la requête que l'on souhaite, +plustot que de définir _comment_ réagir face à notre requête. +Ce système permet d'être beaucoup plus clair sur la forme attendue d'une requête, et de garder une lisibilité satisfaisante peu importe le nombre de +champs que celle-ci contient. + +Nous pouvons ensuite emballer les erreurs de validation dans des `ValidationFail` et `FieldValidationFail`, ce qui permet ensuite d'obtenir +plus de précision sur une erreur, comme le nom du champ qui est invalidé, et qui permet ensuite à nos vues de pouvoir manipuler plus facilement +les erreurs et facilement entourer les champs invalides en rouge, ainsi que d'afficher toutes les erreurs que l'utilisateur a fait, d'un coup. + +### HttpRequest, HttpResponse +Nous avons choisi d'encapsuler les requêtes et les réponses afin de faciliter leur manipulation. +Cela permet de mieux repérer les endroits du code qui manipulent une requête d'un simple tableau, +et de garder à un seul endroit la responsabilitée d'écrire le contenu de la requête vers client. + +`src/App` définit une `ViewHttpResponse`, qui permet aux controlleurs de retourner la vue qu'ils ont choisit. +C'est ensuite à la classe `src/App/App` d'afficher la réponse. + +### index.php +Il y a deux points d'entrés, celui de la WebApp (`public/index.php`), et celui de l'API (`public/api/index.php`). +Ces fichiers ont pour but de définir ce qui va être utilisé dans l'application, comme les routes, la base de donnée, les classes métiers utilisés, +comment gérer l'authentification dans le cadre de l'API, et une session dans le cadre de la web app. + +L'index définit aussi quoi faire lorsque l'application retourne une réponse. Dans les implémentations actuelles, elle délègue simplement +l'affichage dans une classe (`IQBall\App\App` et `IQBall\API\API`). + +### API +Nous avons définit une petite API (`src/Api`) pour nous permettre de faire des actions en arrière plan depuis le front end. +Par exemple, l'API permet de changer le nom d'une tactique, ou de sauvegarder son contenu. +C'est plus pratique de faire des requêtes en arrière-plan plustot que faire une requête directement à la partie WebApp, ce qui +aurait eu pour conséquences de recharger la page + + +## Frontend + +### Utilisation de React + +Notre application est une application de création et de visualisation de stratégies pour des match de basket. +L’éditeur devra comporter un terrain sur lequel il est possible de placer et bouger des pions, représentant les joueurs. +Une stratégie est un arbre composé de plusieurs étapes, une étape étant constituée d’un ensemble de joueurs et d’adversaires sur le terrain, +aillant leur position de départ, et leur position cible, avec des flèches représentant le type de mouvement (dribble, écran, etc) effectué. +les enfants d’une étape, sont d’autres étapes en fonction des cas de figures (si tel joueur fait tel mouvement, ou si tel joueur fait telle passe etc). +Pour rendre le tout agréable à utiliser, il faut que l’interface soit réactive : si l’on bouge un joueur, +il faut que les flèches qui y sont liés bougent aussi, il faut que les joueurs puissent bouger le long des flèches en mode visualiseur etc… + +Le front-end de l’éditeur et du visualiseur étant assez ambitieux, et occupant une place importante du projet, nous avons décidés de l’effectuer en utilisant +le framework React qui rend simple le développement d’interfaces dynamiques, et d’utiliser typescript parce qu’ici on code bien et qu’on impose une type safety a notre code. + diff --git a/public/api/index.php b/public/api/index.php index 076dd11..6f46638 100644 --- a/public/api/index.php +++ b/public/api/index.php @@ -3,11 +3,12 @@ require "../../config.php"; require "../../vendor/autoload.php"; require "../../sql/database.php"; -require "../utils.php"; +require "../../src/index-utils.php"; use IQBall\Api\API; use IQBall\Api\Controller\APIAuthController; use IQBall\Api\Controller\APITacticController; +use IQBall\App\Session\PhpSessionHandle; use IQBall\Core\Action; use IQBall\Core\Connection; use IQBall\Core\Data\Account; @@ -34,4 +35,25 @@ function getRoutes(): AltoRouter { return $router; } -Api::render(API::handleMatch(getRoutes()->match())); +/** + * Defines the way of being authorised through the API + * By checking if an Authorisation header is set, and by expecting its value to be a valid token of an account. + * If the header is not set, fallback to the App's PHP session system, and try to extract the account from it. + * @return Account|null + * @throws Exception + */ +function tryGetAuthorization(): ?Account { + $headers = getallheaders(); + + // If no authorization header is set, try fallback to php session. + if (!isset($headers['Authorization'])) { + $session = PhpSessionHandle::init(); + return $session->getAccount(); + } + + $token = $headers['Authorization']; + $gateway = new AccountGateway(new Connection(get_database())); + return $gateway->getAccountFromToken($token); +} + +Api::render(API::handleMatch(getRoutes()->match(), fn() => tryGetAuthorization())); diff --git a/public/index.php b/public/index.php index 807ec88..01ba967 100644 --- a/public/index.php +++ b/public/index.php @@ -4,8 +4,8 @@ require "../vendor/autoload.php"; require "../config.php"; require "../sql/database.php"; -require "../src/utils.php"; require "../src/App/react-display.php"; +require "../src/index-utils.php"; use IQBall\App\App; use IQBall\App\Controller\AuthController; @@ -13,6 +13,9 @@ use IQBall\App\Controller\EditorController; use IQBall\App\Controller\TeamController; use IQBall\App\Controller\UserController; use IQBall\App\Controller\VisualizerController; +use IQBall\App\Session\MutableSessionHandle; +use IQBall\App\Session\PhpSessionHandle; +use IQBall\App\Session\SessionHandle; use IQBall\App\ViewHttpResponse; use IQBall\Core\Action; use IQBall\Core\Connection; @@ -25,9 +28,6 @@ use IQBall\Core\Http\HttpResponse; use IQBall\Core\Model\AuthModel; use IQBall\Core\Model\TacticModel; use IQBall\Core\Model\TeamModel; -use IQBall\Core\Session\MutableSessionHandle; -use IQBall\Core\Session\PhpSessionHandle; -use IQBall\Core\Session\SessionHandle; use IQBall\Core\Validation\ValidationFail; function getConnection(): Connection { @@ -68,8 +68,8 @@ function getRoutes(): AltoRouter { $ar->map("POST", "/register", Action::noAuth(fn(SessionHandle $s) => getAuthController()->register($_POST, $s))); //user-related - $ar->map("GET", "/home", Action::auth(fn(SessionHandle $s) => getUserController()->home($s))); $ar->map("GET", "/", Action::auth(fn(SessionHandle $s) => getUserController()->home($s))); + $ar->map("GET", "/home", Action::auth(fn(SessionHandle $s) => getUserController()->home($s))); $ar->map("GET", "/settings", Action::auth(fn(SessionHandle $s) => getUserController()->settings($s))); //tactic-related diff --git a/src/Api/API.php b/src/Api/API.php index d266e79..e96b6b6 100644 --- a/src/Api/API.php +++ b/src/Api/API.php @@ -4,12 +4,8 @@ namespace IQBall\Api; use Exception; use IQBall\Core\Action; -use IQBall\Core\Connection; -use IQBall\Core\Data\Account; -use IQBall\Core\Gateway\AccountGateway; use IQBall\Core\Http\HttpResponse; use IQBall\Core\Http\JsonHttpResponse; -use IQBall\Core\Session\PhpSessionHandle; use IQBall\Core\Validation\ValidationFail; class API { @@ -28,12 +24,14 @@ class API { } } + /** - * @param mixed[] $match + * @param array $match + * @param callable $tryGetAuthorization function to return an authorisation object for the given action (if required) * @return HttpResponse * @throws Exception */ - public static function handleMatch(array $match): HttpResponse { + public static function handleMatch(array $match, callable $tryGetAuthorization): HttpResponse { if (!$match) { return new JsonHttpResponse([ValidationFail::notFound("not found")]); } @@ -46,7 +44,7 @@ class API { $auth = null; if ($action->isAuthRequired()) { - $auth = self::tryGetAuthorization(); + $auth = call_user_func($tryGetAuthorization); if ($auth == null) { return new JsonHttpResponse([ValidationFail::unauthorized("Missing or invalid 'Authorization' header.")]); } @@ -54,18 +52,4 @@ class API { return $action->run($match['params'], $auth); } - - private static function tryGetAuthorization(): ?Account { - $headers = getallheaders(); - - // If no authorization header is set, try fallback to php session. - if (!isset($headers['Authorization'])) { - $session = PhpSessionHandle::init(); - return $session->getAccount(); - } - - $token = $headers['Authorization']; - $gateway = new AccountGateway(new Connection(get_database())); - return $gateway->getAccountFromToken($token); - } } diff --git a/src/Api/Controller/APIAuthController.php b/src/Api/Controller/APIAuthController.php index 332f260..5e149a5 100644 --- a/src/Api/Controller/APIAuthController.php +++ b/src/Api/Controller/APIAuthController.php @@ -38,7 +38,7 @@ class APIAuthController { } return new JsonHttpResponse(["authorization" => $account->getToken()]); - }, true); + }); } } diff --git a/src/Api/Controller/APITacticController.php b/src/Api/Controller/APITacticController.php index 63e4811..04f0a50 100644 --- a/src/Api/Controller/APITacticController.php +++ b/src/Api/Controller/APITacticController.php @@ -43,6 +43,6 @@ class APITacticController { } return HttpResponse::fromCode(HttpCodes::OK); - }, true); + }); } } diff --git a/src/App/App.php b/src/App/App.php index 7fa2595..de288e8 100644 --- a/src/App/App.php +++ b/src/App/App.php @@ -2,10 +2,10 @@ namespace IQBall\App; +use IQBall\App\Session\MutableSessionHandle; use IQBall\Core\Action; use IQBall\Core\Http\HttpResponse; use IQBall\Core\Http\JsonHttpResponse; -use IQBall\Core\Session\MutableSessionHandle; use Twig\Environment; use Twig\Error\LoaderError; use Twig\Error\RuntimeError; diff --git a/src/App/Control.php b/src/App/Control.php index 3dea9b1..f3860ec 100644 --- a/src/App/Control.php +++ b/src/App/Control.php @@ -16,22 +16,17 @@ class Control { * @param array $schema an array of `fieldName => Validators` which represents the request object schema * @param callable(HttpRequest): HttpResponse $run the callback to run if the request is valid according to the given schema. * THe callback must accept an HttpRequest, and return an HttpResponse object. - * @param bool $errorInJson if set to true, the returned response, in case of errors, will be a JsonHttpResponse, instead - * of the ViewHttpResponse for an error view. * @return HttpResponse */ - public static function runChecked(array $schema, callable $run, bool $errorInJson): HttpResponse { + public static function runChecked(array $schema, callable $run): HttpResponse { $request_body = file_get_contents('php://input'); $payload_obj = json_decode($request_body); if (!$payload_obj instanceof \stdClass) { $fail = new ValidationFail("bad-payload", "request body is not a valid json object"); - if ($errorInJson) { - return new JsonHttpResponse([$fail, HttpCodes::BAD_REQUEST]); - } return ViewHttpResponse::twig("error.html.twig", ["failures" => [$fail]], HttpCodes::BAD_REQUEST); } $payload = get_object_vars($payload_obj); - return self::runCheckedFrom($payload, $schema, $run, $errorInJson); + return self::runCheckedFrom($payload, $schema, $run); } /** @@ -40,18 +35,13 @@ class Control { * @param array $schema an array of `fieldName => Validators` which represents the request object schema * @param callable(HttpRequest): HttpResponse $run the callback to run if the request is valid according to the given schema. * THe callback must accept an HttpRequest, and return an HttpResponse object. - * @param bool $errorInJson if set to true, the returned response, in case of errors, will be a JsonHttpResponse, instead - * of the ViewHttpResponse for an error view. * @return HttpResponse */ - public static function runCheckedFrom(array $data, array $schema, callable $run, bool $errorInJson): HttpResponse { + public static function runCheckedFrom(array $data, array $schema, callable $run): HttpResponse { $fails = []; $request = HttpRequest::from($data, $fails, $schema); if (!empty($fails)) { - if ($errorInJson) { - return new JsonHttpResponse($fails, HttpCodes::BAD_REQUEST); - } return ViewHttpResponse::twig("error.html.twig", ['failures' => $fails], HttpCodes::BAD_REQUEST); } diff --git a/src/App/Controller/AuthController.php b/src/App/Controller/AuthController.php index 9e790e9..dfc5f52 100644 --- a/src/App/Controller/AuthController.php +++ b/src/App/Controller/AuthController.php @@ -2,12 +2,11 @@ namespace IQBall\App\Controller; +use IQBall\App\Session\MutableSessionHandle; +use IQBall\App\ViewHttpResponse; use IQBall\Core\Http\HttpRequest; use IQBall\Core\Http\HttpResponse; -use IQBall\App\ViewHttpResponse; use IQBall\Core\Model\AuthModel; -use IQBall\Core\Session\MutableSessionHandle; -use IQBall\Core\Validation\ValidationFail; use IQBall\Core\Validation\Validators; class AuthController { diff --git a/src/App/Controller/EditorController.php b/src/App/Controller/EditorController.php index 6724ced..bba3214 100644 --- a/src/App/Controller/EditorController.php +++ b/src/App/Controller/EditorController.php @@ -2,13 +2,13 @@ namespace IQBall\App\Controller; +use IQBall\App\Session\SessionHandle; +use IQBall\App\Validator\TacticValidator; +use IQBall\App\ViewHttpResponse; use IQBall\Core\Data\TacticInfo; use IQBall\Core\Http\HttpCodes; -use IQBall\Core\Http\HttpResponse; -use IQBall\App\ViewHttpResponse; use IQBall\Core\Model\TacticModel; -use IQBall\Core\Session\SessionHandle; -use IQBall\Core\Validator\TacticValidator; +use IQBall\Core\Validation\ValidationFail; class EditorController { private TacticModel $model; @@ -44,7 +44,7 @@ class EditorController { public function openEditor(int $id, SessionHandle $session): ViewHttpResponse { $tactic = $this->model->get($id); - $failure = TacticValidator::validateAccess($tactic, $id, $session->getAccount()->getId()); + $failure = TacticValidator::validateAccess($id, $tactic, $session->getAccount()->getId()); if ($failure != null) { return ViewHttpResponse::twig('error.html.twig', ['failures' => [$failure]], HttpCodes::NOT_FOUND); diff --git a/src/App/Controller/TeamController.php b/src/App/Controller/TeamController.php index deaf61b..b2c0ea9 100644 --- a/src/App/Controller/TeamController.php +++ b/src/App/Controller/TeamController.php @@ -2,11 +2,11 @@ namespace IQBall\App\Controller; +use IQBall\App\Session\SessionHandle; +use IQBall\App\ViewHttpResponse; use IQBall\Core\Http\HttpRequest; use IQBall\Core\Http\HttpResponse; -use IQBall\App\ViewHttpResponse; use IQBall\Core\Model\TeamModel; -use IQBall\Core\Session\SessionHandle; use IQBall\Core\Validation\FieldValidationFail; use IQBall\Core\Validation\Validators; diff --git a/src/App/Controller/UserController.php b/src/App/Controller/UserController.php index b7cef0b..d6f9f89 100644 --- a/src/App/Controller/UserController.php +++ b/src/App/Controller/UserController.php @@ -2,10 +2,9 @@ namespace IQBall\App\Controller; -use IQBall\Core\Http\HttpResponse; +use IQBall\App\Session\SessionHandle; use IQBall\App\ViewHttpResponse; use IQBall\Core\Model\TacticModel; -use IQBall\Core\Session\SessionHandle; class UserController { private TacticModel $tactics; diff --git a/src/App/Controller/VisualizerController.php b/src/App/Controller/VisualizerController.php index 48e1168..271c4e9 100644 --- a/src/App/Controller/VisualizerController.php +++ b/src/App/Controller/VisualizerController.php @@ -2,12 +2,12 @@ namespace IQBall\App\Controller; +use IQBall\App\Session\SessionHandle; +use IQBall\App\Validator\TacticValidator; +use IQBall\App\ViewHttpResponse; use IQBall\Core\Http\HttpCodes; use IQBall\Core\Http\HttpResponse; -use IQBall\App\ViewHttpResponse; use IQBall\Core\Model\TacticModel; -use IQBall\Core\Session\SessionHandle; -use IQBall\Core\Validator\TacticValidator; class VisualizerController { private TacticModel $tacticModel; @@ -28,7 +28,7 @@ class VisualizerController { public function openVisualizer(int $id, SessionHandle $session): HttpResponse { $tactic = $this->tacticModel->get($id); - $failure = TacticValidator::validateAccess($tactic, $id, $session->getAccount()->getId()); + $failure = TacticValidator::validateAccess($id, $tactic, $session->getAccount()->getId()); if ($failure != null) { return ViewHttpResponse::twig('error.html.twig', ['failures' => [$failure]], HttpCodes::NOT_FOUND); diff --git a/src/Core/Session/MutableSessionHandle.php b/src/App/Session/MutableSessionHandle.php similarity index 93% rename from src/Core/Session/MutableSessionHandle.php rename to src/App/Session/MutableSessionHandle.php index 1d7ae86..9ef23c0 100644 --- a/src/Core/Session/MutableSessionHandle.php +++ b/src/App/Session/MutableSessionHandle.php @@ -1,6 +1,6 @@ getOwnerId() != $ownerId) { return ValidationFail::unauthorized("Vous ne pouvez pas accéder à cette tactique."); } - return null; } + } diff --git a/src/Core/Validation/ComposedValidator.php b/src/Core/Validation/ComposedValidator.php index fcf81d9..58f4910 100644 --- a/src/Core/Validation/ComposedValidator.php +++ b/src/Core/Validation/ComposedValidator.php @@ -17,7 +17,10 @@ class ComposedValidator extends Validator { public function validate(string $name, $val): array { $firstFailures = $this->first->validate($name, $val); - $thenFailures = $this->then->validate($name, $val); + $thenFailures = []; + if (empty($firstFailures)) { + $thenFailures = $this->then->validate($name, $val); + } return array_merge($firstFailures, $thenFailures); } } diff --git a/src/Core/Validation/Validator.php b/src/Core/Validation/Validator.php index 8624bfe..d1761da 100644 --- a/src/Core/Validation/Validator.php +++ b/src/Core/Validation/Validator.php @@ -13,7 +13,7 @@ abstract class Validator { /** * Creates a validator composed of this validator, and given validator - * @param Validator $other the second validator to chain with + * @param Validator $other the second validator to validate if this validator succeeded * @return Validator a composed validator */ public function then(Validator $other): Validator { diff --git a/src/utils.php b/src/index-utils.php similarity index 100% rename from src/utils.php rename to src/index-utils.php