From a1910d1167144b61a0eef75266ea467b155edfd5 Mon Sep 17 00:00:00 2001 From: Override-6 Date: Sat, 11 Nov 2023 18:15:38 +0100 Subject: [PATCH] move validations in the controllers, make controllers return HttpResponses and use HttpRequests --- public/api/index.php | 18 +++-- public/index.php | 44 +++++++++--- src/Api/TacticEndpoint.php | 78 ---------------------- src/Controller/Api/APITacticController.php | 55 +++++++++++++++ src/Controller/Control.php | 31 +++++++++ src/Controller/EditorController.php | 26 +++++--- src/Controller/SampleFormController.php | 54 ++++++++------- src/Http/HttpRequest.php | 63 +++++++++++++++++ src/Http/HttpResponse.php | 24 +++++++ src/Http/JsonHttpResponse.php | 23 +++++++ src/Http/ViewHttpResponse.php | 49 ++++++++++++++ src/Model/TacticModel.php | 23 +++---- src/Validation/ComposedValidator.php | 24 +++++++ src/Validation/FieldValidationFail.php | 6 +- src/Validation/SimpleFunctionValidator.php | 12 ++-- src/Validation/Validation.php | 6 +- src/Validation/Validator.php | 15 ++++- src/Validation/Validators.php | 21 +++--- 18 files changed, 402 insertions(+), 170 deletions(-) delete mode 100644 src/Api/TacticEndpoint.php create mode 100644 src/Controller/Api/APITacticController.php create mode 100644 src/Controller/Control.php create mode 100644 src/Http/HttpRequest.php create mode 100644 src/Http/HttpResponse.php create mode 100644 src/Http/JsonHttpResponse.php create mode 100644 src/Http/ViewHttpResponse.php create mode 100644 src/Validation/ComposedValidator.php diff --git a/public/api/index.php b/public/api/index.php index 52e1f97..b6327e1 100644 --- a/public/api/index.php +++ b/public/api/index.php @@ -6,8 +6,10 @@ require "../../sql/database.php"; require "../utils.php"; use App\Connexion; -use App\Api\TacticEndpoint; +use App\Controller\Api\APITacticController; use App\Gateway\TacticInfoGateway; +use App\Http\JsonHttpResponse; +use App\Http\ViewHttpResponse; use App\Model\TacticModel; $con = new Connexion(get_database()); @@ -15,7 +17,7 @@ $con = new Connexion(get_database()); $router = new AltoRouter(); $router->setBasePath(get_public_path() . "/api"); -$tacticEndpoint = new TacticEndpoint(new TacticModel(new TacticInfoGateway($con))); +$tacticEndpoint = new APITacticController(new TacticModel(new TacticInfoGateway($con))); $router->map("POST", "/tactic/[i:id]/edit/name", fn(int $id) => $tacticEndpoint->updateName($id)); $router->map("GET", "/tactic/[i:id]", fn(int $id) => $tacticEndpoint->getTacticInfo($id)); $router->map("POST", "/tactic/new", fn() => $tacticEndpoint->newTactic()); @@ -28,5 +30,13 @@ if ($match == null) { exit(1); } -header('Content-type: application/json'); -call_user_func_array($match['target'], $match['params']); \ No newline at end of file +$response = call_user_func_array($match['target'], $match['params']); + +http_response_code($response->getCode()); + +if ($response instanceof JsonHttpResponse) { + header('Content-type: application/json'); + echo $response->getJson(); +} else if ($response instanceof ViewHttpResponse) { + throw new Exception("API returned a view http response."); +} \ No newline at end of file diff --git a/public/index.php b/public/index.php index b4b3f9a..caa8a2e 100644 --- a/public/index.php +++ b/public/index.php @@ -5,15 +5,15 @@ require "../config.php"; require "../sql/database.php"; require "utils.php"; -use \Twig\Loader\FilesystemLoader; use App\Connexion; -use App\Controller\SampleFormController; use App\Controller\EditorController; - +use App\Controller\SampleFormController; use App\Gateway\FormResultGateway; use App\Gateway\TacticInfoGateway; - +use App\Http\JsonHttpResponse; +use App\Http\ViewHttpResponse; use App\Model\TacticModel; +use Twig\Loader\FilesystemLoader; $loader = new FilesystemLoader('../src/Views/'); @@ -29,12 +29,12 @@ $router->setBasePath($basePath); $sampleFormController = new SampleFormController(new FormResultGateway($con), $twig); $editorController = new EditorController(new TacticModel(new TacticInfoGateway($con))); -$router->map("GET", "/", fn() => $sampleFormController->displayForm()); -$router->map("POST", "/submit", fn() => $sampleFormController->submitForm($_POST)); +$router->map("GET", "/", fn() => $sampleFormController->displayFormReact()); +$router->map("POST", "/submit", fn() => $sampleFormController->submitFormReact($_POST)); $router->map("GET", "/twig", fn() => $sampleFormController->displayFormTwig()); $router->map("POST", "/submit-twig", fn() => $sampleFormController->submitFormTwig($_POST)); $router->map("GET", "/tactic/new", fn() => $editorController->makeNew()); -$router->map("GET", "/tactic/[i:id]/edit", fn(int $id) => $editorController->edit($id)); +$router->map("GET", "/tactic/[i:id]/edit", fn(int $id) => $editorController->openEditorFor($id)); $match = $router->match(); @@ -42,7 +42,33 @@ if ($match == null) { // TODO redirect to a 404 not found page instead (issue #1) http_response_code(404); echo "Page non trouvée"; - exit(1); + return; } -call_user_func_array($match['target'], $match['params']); +$response = call_user_func_array($match['target'], $match['params']); + +http_response_code($response->getCode()); + +if ($response instanceof ViewHttpResponse) { + $file = $response->getFile(); + $args = $response->getArguments(); + + switch ($response->getViewKind()) { + case ViewHttpResponse::REACT_VIEW: + send_react_front($file, $args); + break; + case ViewHttpResponse::TWIG_VIEW: + try { + $twig->display($file, $args); + } catch (\Twig\Error\RuntimeError|\Twig\Error\SyntaxError $e) { + http_response_code(500); + echo "There was an error rendering your view, please refer to an administrator.\nlogs date: " . date("YYYD, d M Y H:i:s"); + throw e; + } + break; + } + +} else if ($response instanceof JsonHttpResponse) { + header('Content-type: application/json'); + echo $response->getJson(); +} \ No newline at end of file diff --git a/src/Api/TacticEndpoint.php b/src/Api/TacticEndpoint.php deleted file mode 100644 index 9c1ee8c..0000000 --- a/src/Api/TacticEndpoint.php +++ /dev/null @@ -1,78 +0,0 @@ -model = $model; - } - - - public function updateName(int $tactic_id): void { - $request_body = file_get_contents('php://input'); - $data = json_decode($request_body); - - if (!isset($data->name)) { - http_response_code(HttpCodes::BAD_REQUEST); - echo "missing 'name'"; - return; - } - - $fails = []; - - $this->model->updateName($fails, $tactic_id, $data->name); - if (!empty($fails)) { - http_response_code(HttpCodes::PRECONDITION_FAILED); - echo json_encode($fails); - } - } - - public function newTactic(): void { - $request_body = file_get_contents('php://input'); - $data = json_decode($request_body); - - $initial_name = $data->name; - - if (!isset($data->name)) { - http_response_code(HttpCodes::BAD_REQUEST); - echo "missing 'name'"; - return; - } - - $fails = []; - $tactic = $this->model->makeNew($fails, $initial_name); - - if (!empty($fails)) { - http_response_code(HttpCodes::PRECONDITION_FAILED); - echo json_encode($fails); - return; - } - - $id = $tactic->getId(); - echo "{id: $id}"; - } - - public function getTacticInfo(int $id): void { - $tactic_info = $this->model->get($id); - - if ($tactic_info == null) { - http_response_code(HttpCodes::NOT_FOUND); - return; - } - - echo json_encode($tactic_info); - } - -} \ No newline at end of file diff --git a/src/Controller/Api/APITacticController.php b/src/Controller/Api/APITacticController.php new file mode 100644 index 0000000..a1f7767 --- /dev/null +++ b/src/Controller/Api/APITacticController.php @@ -0,0 +1,55 @@ +model = $model; + } + + public function updateName(int $tactic_id): HttpResponse { + return Control::runChecked([ + "name" => [Validators::userString(32)] + ], function (HttpRequest $request) use ($tactic_id) { + $this->model->updateName($tactic_id, $request["name"]); + return HttpResponse::fromCode(HttpCodes::OK); + }); + } + + public function newTactic(): HttpResponse { + return Control::runChecked([ + "name" => [Validators::userString(32)] + ], function (HttpRequest $request) { + $tactic = $this->model->makeNew($request["name"]); + $id = $tactic->getId(); + return new JsonHttpResponse(["id" => $id]); + }); + } + + public function getTacticInfo(int $id): HttpResponse { + $tactic_info = $this->model->get($id); + + if ($tactic_info == null) { + return new JsonHttpResponse("could not find tactic #$id", HttpCodes::NOT_FOUND); + } + + return new JsonHttpResponse($tactic_info); + } + +} \ No newline at end of file diff --git a/src/Controller/Control.php b/src/Controller/Control.php new file mode 100644 index 0000000..af1a38b --- /dev/null +++ b/src/Controller/Control.php @@ -0,0 +1,31 @@ +model = $model; } - private function openEditor(TacticInfo $tactic) { - send_react_front("views/Editor.tsx", ["name" => $tactic->getName(), "id" => $tactic->getId()]); + private function openEditor(TacticInfo $tactic): HttpResponse { + return ViewHttpResponse::react("views/Editor.tsx", ["name" => $tactic->getName(), "id" => $tactic->getId()]); } - public function makeNew() { + public function makeNew(): HttpResponse { $tactic = $this->model->makeNewDefault(); - $this->openEditor($tactic); + return $this->openEditor($tactic); } - public function edit(int $id) { + /** + * returns an editor view for a given tactic + * @param int $id the targeted tactic identifier + * @return HttpResponse + */ + public function openEditorFor(int $id): HttpResponse { $tactic = $this->model->get($id); if ($tactic == null) { - http_response_code(404); - echo "la tactique " . $id . " n'existe pas"; - return; + return new JsonHttpResponse("la tactique " . $id . " n'existe pas", HttpCodes::NOT_FOUND); } - $this->openEditor($tactic); + return $this->openEditor($tactic); } } \ No newline at end of file diff --git a/src/Controller/SampleFormController.php b/src/Controller/SampleFormController.php index ad77d62..b65b265 100644 --- a/src/Controller/SampleFormController.php +++ b/src/Controller/SampleFormController.php @@ -3,52 +3,50 @@ namespace App\Controller; require_once __DIR__ . "/../react-display.php"; + use App\Gateway\FormResultGateway; -use \Twig\Environment; -use Twig\Error\LoaderError; -use Twig\Error\RuntimeError; -use Twig\Error\SyntaxError; +use App\Http\HttpRequest; +use App\Http\HttpResponse; +use App\Http\ViewHttpResponse; +use App\Validation\Validators; class SampleFormController { private FormResultGateway $gateway; - private Environment $twig; /** * @param FormResultGateway $gateway */ - public function __construct(FormResultGateway $gateway, Environment $twig) - { + public function __construct(FormResultGateway $gateway) { $this->gateway = $gateway; - $this->twig = $twig; } - public function displayForm() { - send_react_front("views/SampleForm.tsx", []); + public function displayFormReact(): HttpResponse { + return ViewHttpResponse::react("views/SampleForm.tsx", []); + } + + public function displayFormTwig(): HttpResponse { + return ViewHttpResponse::twig('sample_form.html.twig', []); } - public function submitForm(array $request) { - $this->gateway->insert($request["name"], $request["description"]); - $results = ["results" => $this->gateway->listResults()]; - send_react_front("views/DisplayResults.tsx", $results); + private function submitForm(array $form, callable $response): HttpResponse { + return Control::runCheckedFrom($form, [ + "name" => [Validators::userString(32)], + "description" => [Validators::userString(512)] + ], function (HttpRequest $req) use ($response) { + $this->gateway->insert($req["name"], $req["description"]); + $results = ["results" => $this->gateway->listResults()]; + + return call_user_func_array($response, [$results]); + }); } - public function displayFormTwig() { - try { - echo $this->twig->render('sample_form.html.twig', []); - } catch (LoaderError | RuntimeError | SyntaxError $e) { - echo "Twig error: $e"; - } + public function submitFormTwig(array $form): HttpResponse { + return $this->submitForm($form, fn(array $results) => ViewHttpResponse::twig('display_results.html.twig', $results)); } - public function submitFormTwig(array $request) { - $this->gateway->insert($request["name"], $request["description"]); - try { - $results = $this->gateway->listResults(); - echo $this->twig->render('display_results.html.twig', ['results' => $results]); - } catch (LoaderError | RuntimeError | SyntaxError $e) { - echo "Twig error: $e"; - } + public function submitFormReact(array $form): HttpResponse { + return $this->submitForm($form, fn(array $results) => ViewHttpResponse::react('views/DisplayResults.tsx', $results)); } } \ No newline at end of file diff --git a/src/Http/HttpRequest.php b/src/Http/HttpRequest.php new file mode 100644 index 0000000..ce75d76 --- /dev/null +++ b/src/Http/HttpRequest.php @@ -0,0 +1,63 @@ +data = $data; + } + + /** + * Creates a new HttpRequest instance, and ensures that the given request data validates the given schema. + * This is a simple function that only supports flat schemas (non-composed, the data must only be a k/v array pair.) + * @param array $request the request's data + * @param array $fails a reference to a failure array, that will contain the reported validation failures. + * @param array $schema the schema to satisfy. a schema is a simple array with a string key (which is the top-level field name), and a set of validators + * @return HttpRequest|null the built HttpRequest instance, or null if a field is missing, or if any of the schema validator failed + */ + public static function from(array $request, array &$fails, array $schema): ?HttpRequest { + $failure = false; + foreach ($schema as $fieldName => $fieldValidators) { + if (!isset($request[$fieldName])) { + $fails[] = FieldValidationFail::missing($fieldName); + $failure = true; + continue; + } + $failure |= Validation::validate($request[$fieldName], $fieldName, $fails, ...$fieldValidators); + } + + if ($failure) { + return null; + } + return new HttpRequest($request); + } + + public static function fromPayload(array &$fails, array $schema): ?HttpRequest { + $request_body = file_get_contents('php://input'); + $data = json_decode($request_body); + return self::from($data, $fails, $schema); + } + + public function offsetExists($offset): bool { + return isset($this->data[$offset]); + } + + public function offsetGet($offset) { + return $this->data[$offset]; + } + + public function offsetSet($offset, $value) { + throw new Exception("requests are immutable objects."); + } + + public function offsetUnset($offset) { + throw new Exception("requests are immutable objects."); + } +} \ No newline at end of file diff --git a/src/Http/HttpResponse.php b/src/Http/HttpResponse.php new file mode 100644 index 0000000..9f081a5 --- /dev/null +++ b/src/Http/HttpResponse.php @@ -0,0 +1,24 @@ +code = $code; + } + + public function getCode(): int { + return $this->code; + } + + public static function fromCode(int $code): HttpResponse { + return new HttpResponse($code); + } + +} \ No newline at end of file diff --git a/src/Http/JsonHttpResponse.php b/src/Http/JsonHttpResponse.php new file mode 100644 index 0000000..1c30017 --- /dev/null +++ b/src/Http/JsonHttpResponse.php @@ -0,0 +1,23 @@ +payload = $payload; + } + + public function getJson() { + return json_encode($this->payload); + } + +} \ No newline at end of file diff --git a/src/Http/ViewHttpResponse.php b/src/Http/ViewHttpResponse.php new file mode 100644 index 0000000..6950559 --- /dev/null +++ b/src/Http/ViewHttpResponse.php @@ -0,0 +1,49 @@ +kind = $kind; + $this->file = $file; + $this->arguments = $arguments; + } + + public function getViewKind(): int { + return $this->kind; + } + + public function getFile(): string { + return $this->file; + } + + public function getArguments(): array { + return $this->arguments; + } + + public static function twig(string $file, array $arguments, int $code = HttpCodes::OK): ViewHttpResponse { + return new ViewHttpResponse(self::TWIG_VIEW, $file, $arguments, $code); + } + + public static function react(string $file, array $arguments, int $code = HttpCodes::OK): ViewHttpResponse { + return new ViewHttpResponse(self::REACT_VIEW, $file, $arguments, $code); + } + +} \ No newline at end of file diff --git a/src/Model/TacticModel.php b/src/Model/TacticModel.php index 7f31572..c0b1ffe 100644 --- a/src/Model/TacticModel.php +++ b/src/Model/TacticModel.php @@ -4,9 +4,6 @@ namespace App\Model; use App\Data\TacticInfo; use App\Gateway\TacticInfoGateway; -use App\Validation\ValidationFail; -use App\Validation\Validation; -use App\Validation\Validators; class TacticModel { @@ -22,12 +19,8 @@ class TacticModel { $this->tactics = $tactics; } - public function makeNew(array &$fails, string $name): ?TacticInfo { - $failure = Validation::validate($name, "name", $fails, Validators::nonEmpty(), Validators::noInvalidChars()); - if ($failure) { - return null; - } - $this->tactics->insert($name); + public function makeNew(string $name): TacticInfo { + return $this->tactics->insert($name); } public function makeNewDefault(): ?TacticInfo { @@ -47,15 +40,15 @@ class TacticModel { * Update the name of a tactic * @param int $id the tactic identifier * @param string $name the new name to set + * @return true if the update was done successfully */ - public function updateName(array &$fails, int $id, string $name): void { - $failure = Validation::validate($name, "name", $fails, Validators::nonEmpty(), Validators::noInvalidChars()); - + public function updateName(int $id, string $name): bool { if ($this->tactics->get($id) == null) { - $fails[] = ValidationFail::notFound("$id is an unknown tactic identifier"); - } else if (!$failure) { - $this->tactics->updateName($id, $name); + return false; } + + $this->tactics->updateName($id, $name); + return true; } } \ No newline at end of file diff --git a/src/Validation/ComposedValidator.php b/src/Validation/ComposedValidator.php new file mode 100644 index 0000000..418b1ed --- /dev/null +++ b/src/Validation/ComposedValidator.php @@ -0,0 +1,24 @@ +first = $first; + $this->then = $then; + } + + public function validate(string $name, $val): array { + $firstFailures = $this->first->validate($name, $val); + $thenFailures = $this->then->validate($name, $val); + return array_merge($firstFailures, $thenFailures); + } +} \ No newline at end of file diff --git a/src/Validation/FieldValidationFail.php b/src/Validation/FieldValidationFail.php index cae1496..404a497 100644 --- a/src/Validation/FieldValidationFail.php +++ b/src/Validation/FieldValidationFail.php @@ -18,12 +18,10 @@ class FieldValidationFail extends ValidationFail { $this->fieldName = $fieldName; } - public function getFieldName(): string { return $this->fieldName; } - public static function invalidChars(string $fieldName): FieldValidationFail { return new FieldValidationFail($fieldName, "field contains illegal chars"); } @@ -32,6 +30,10 @@ class FieldValidationFail extends ValidationFail { return new FieldValidationFail($fieldName, "field is empty"); } + public static function missing(string $fieldName): FieldValidationFail { + return new FieldValidationFail($fieldName, "field is missing"); + } + public function jsonSerialize() { return ["field" => $this->fieldName, "message" => $this->getMessage()]; } diff --git a/src/Validation/SimpleFunctionValidator.php b/src/Validation/SimpleFunctionValidator.php index 4282916..101dfe6 100644 --- a/src/Validation/SimpleFunctionValidator.php +++ b/src/Validation/SimpleFunctionValidator.php @@ -5,24 +5,24 @@ namespace App\Validation; /** * A simple validator that takes a predicate and an error factory */ -class SimpleFunctionValidator implements Validator { +class SimpleFunctionValidator extends Validator { private $predicate; private $error_factory; /** * @param callable $predicate a function predicate with signature: `(string) => bool`, to validate the given string - * @param callable $error_factory a factory function with signature `(string) => Error)` to emit error when the predicate fails + * @param callable $errors_factory a factory function with signature `(string) => array` to emit failures when the predicate fails */ - public function __construct(callable $predicate, callable $error_factory) { + public function __construct(callable $predicate, callable $errors_factory) { $this->predicate = $predicate; - $this->error_factory = $error_factory; + $this->error_factory = $errors_factory; } - public function validate(string $name, $val): ?ValidationFail { + public function validate(string $name, $val): array { if (!call_user_func_array($this->predicate, [$val])) { return call_user_func_array($this->error_factory, [$name]); } - return null; + return []; } } \ No newline at end of file diff --git a/src/Validation/Validation.php b/src/Validation/Validation.php index da5dadd..659c8b0 100644 --- a/src/Validation/Validation.php +++ b/src/Validation/Validation.php @@ -11,16 +11,16 @@ class Validation { * Validate a value from validators, appending failures in the given errors array. * @param mixed $val the value to validate * @param string $val_name the name of the value - * @param array $errors array to push when a validator fails + * @param array $failures array to push when a validator fails * @param Validator ...$validators given validators * @return bool true if any of the given validators did fail */ - public static function validate($val, string $val_name, array &$errors, Validator...$validators): bool { + public static function validate($val, string $val_name, array &$failures, Validator...$validators): bool { $had_errors = false; foreach ($validators as $validator) { $error = $validator->validate($val_name, $val); if ($error != null) { - $errors[] = $error; + $failures[] = $error; $had_errors = true; } } diff --git a/src/Validation/Validator.php b/src/Validation/Validator.php index b7c77ce..6cdafb9 100644 --- a/src/Validation/Validator.php +++ b/src/Validation/Validator.php @@ -2,14 +2,23 @@ namespace App\Validation; -interface Validator { +abstract class Validator { /** * validates a variable string * @param string $name the name of the tested value * @param mixed $val the value to validate - * @return ValidationFail|null the error if the validator did fail, or null if it succeeded + * @return array the errors the validator has reported */ - public function validate(string $name, $val): ?ValidationFail; + public abstract function validate(string $name, $val): array; + + /** + * Creates a validator composed of this validator, and given validator + * @param Validator $other the second validator to chain with + * @return Validator a composed validator + */ + public function then(Validator $other): Validator { + return new ComposedValidator($this, $other); + } } \ No newline at end of file diff --git a/src/Validation/Validators.php b/src/Validation/Validators.php index 3bebb8c..ebf0f80 100644 --- a/src/Validation/Validators.php +++ b/src/Validation/Validators.php @@ -7,27 +7,26 @@ namespace App\Validation; */ class Validators { - /** - * @return Validator a validator that validates strings that does not contain invalid chars such as `<` and `>` + * @return Validator a validator that validates non-empty strings */ - public static function noInvalidChars(): Validator { + public static function nonEmpty(): Validator { return new SimpleFunctionValidator( - fn($str) => !filter_var($str, FILTER_VALIDATE_REGEXP, ['options' => ["regexp" => "/[<>]/"]]), - fn(string $name) => FieldValidationFail::invalidChars($name) + fn($str) => !empty($str), + fn(string $name) => [FieldValidationFail::empty($name)] ); } - /** - * @return Validator a validator that validates non-empty strings - */ - public static function nonEmpty(): Validator { + public static function shorterThan(int $limit): Validator { return new SimpleFunctionValidator( - fn($str) => !empty($str), - fn(string $name) => FieldValidationFail::empty($name) + fn(string $str) => strlen($str) <= $limit, + fn(string $name) => [new FieldValidationFail($name, "field is longer than $limit chars.")] ); } + public static function userString(int $maxLen): Validator { + return self::nonEmpty()->then(self::shorterThan($maxLen)); + } }