From 3b033675e52ed68ba99148245b75974ade47627a Mon Sep 17 00:00:00 2001 From: Julien Chaffraix Date: Wed, 18 Dec 2024 19:19:07 -0800 Subject: [PATCH] Type api/* The first step was to rework the ApiRouter to clearly state that it was using a trie and add an appropriate TrieNode. This helped clean up the types a bit. Typed the handlers/validators though their return value is anything that `json_encode` can consume (so anything effectively :-)). --- SETUP/tests/unittests/ApiTest.php | 12 +++--- api/ApiRouter.inc | 69 ++++++++++++++++++++----------- api/v1_projects.inc | 50 +++++++++++----------- api/v1_queues.inc | 6 +-- api/v1_validators.inc | 10 ++--- pinc/Project.inc | 4 +- 6 files changed, 87 insertions(+), 64 deletions(-) diff --git a/SETUP/tests/unittests/ApiTest.php b/SETUP/tests/unittests/ApiTest.php index bcf0d94162..4f3bf64a14 100644 --- a/SETUP/tests/unittests/ApiTest.php +++ b/SETUP/tests/unittests/ApiTest.php @@ -116,7 +116,7 @@ public function test_get_invalid_round_stats(): void $this->expectExceptionCode(103); $path = "v1/stats/site/rounds/P4"; - $query_params = ""; + $query_params = []; $router = ApiRouter::get_router(); $_SERVER["REQUEST_METHOD"] = "GET"; $router->route($path, $query_params); @@ -128,7 +128,7 @@ public function test_get_invalid_page_data(): void $project = $this->_create_project(); $path = "v1/projects/$project->projectid/pages/999.png/pagerounds/P1"; - $query_params = ""; + $query_params = []; $router = ApiRouter::get_router(); $_SERVER["REQUEST_METHOD"] = "GET"; $router->route($path, $query_params); @@ -142,7 +142,7 @@ public function test_get_invalid_pageround_data(): void $this->add_page($project, "001"); // P0 is not a valid round $path = "v1/projects/$project->projectid/pages/001.png/pagerounds/P0"; - $query_params = ""; + $query_params = []; $router = ApiRouter::get_router(); $_SERVER["REQUEST_METHOD"] = "GET"; $router->route($path, $query_params); @@ -153,7 +153,7 @@ public function test_get_valid_pageround_data(): void $project = $this->_create_project(); $this->add_page($project, "001"); $path = "v1/projects/$project->projectid/pages/001.png/pagerounds/OCR"; - $query_params = ""; + $query_params = []; $router = ApiRouter::get_router(); $_SERVER["REQUEST_METHOD"] = "GET"; $result = $router->route($path, $query_params); @@ -168,7 +168,7 @@ public function test_create_project_unauthorised(): void $this->expectExceptionCode(3); $path = "v1/projects"; - $query_params = ""; + $query_params = []; $router = ApiRouter::get_router(); $_SERVER["REQUEST_METHOD"] = "POST"; $router->route($path, $query_params); @@ -181,7 +181,7 @@ public function test_create_project_no_data(): void $pguser = $this->TEST_USERNAME_PM; $path = "v1/projects"; - $query_params = ""; + $query_params = []; $router = ApiRouter::get_router(); $_SERVER["REQUEST_METHOD"] = "POST"; $router->route($path, $query_params); diff --git a/api/ApiRouter.inc b/api/ApiRouter.inc index 022ec494f9..a41e035572 100644 --- a/api/ApiRouter.inc +++ b/api/ApiRouter.inc @@ -4,17 +4,34 @@ include_once("exceptions.inc"); // Raise exceptions on assert failures ini_set("assert.exception", 1); +/** + * We use a trie to match the path to its handler(s). + * + * Handlers contains the individual handlers, keyed by the method. + */ +class TrieNode +{ + /** @var array */ + public array $children; + /** @var array */ + public array $handlers; +} + class ApiRouter { - private $_url_map = []; - private $_validators = []; + private TrieNode $root; + /** @var array */ + private $_validators; - public function add_route($method, $url, $function) + public function __construct() { - // Confirm the function is defined or raise an assert exception - assert(function_exists($function), "$function not defined"); + $this->root = new TrieNode(); + $this->_validators = []; + } - $url_map = &$this->_url_map; + public function add_route(string $method, string $url, callable $function): void + { + $node = $this->root; $parts = explode("/", $url); foreach ($parts as $part) { // If this is a param placeholder, confirm there is a validator @@ -25,47 +42,50 @@ class ApiRouter "No validator specified for $part" ); } - if (!isset($url_map[$part])) { - $url_map[$part] = []; + if (!isset($node->children[$part])) { + $node->children[$part] = new TrieNode(); } - $url_map = &$url_map[$part]; + $node = $node->children[$part]; } - $url_map["endpoint"][$method] = $function; + $node->handlers[$method] = $function; } - public function route($url, $query_params) + /** @return mixed */ + public function route(string $url, array $query_params) { - $url_map = &$this->_url_map; + $node = $this->root; $data = []; $parts = explode("/", $url); foreach ($parts as $part) { - if (isset($url_map[$part])) { - $url_map = &$url_map[$part]; + $next_node = $node->children[$part] ?? null; + if ($next_node) { + $node = $next_node; } else { - [$param_name, $validator] = $this->get_validator($url_map); - $url_map = &$url_map[$param_name]; + [$param_name, $validator] = $this->get_validator($node); + $node = $node->children[$param_name]; $data[$param_name] = $validator($part, $data); } } - if (!isset($url_map["endpoint"])) { + if (empty($node->handlers)) { throw new InvalidAPI(); } $method = $_SERVER["REQUEST_METHOD"]; - if (!isset($url_map["endpoint"][$method])) { + $handler = $node->handlers[$method] ?? null; + if (!$handler) { throw new MethodNotAllowed(); } - $function = $url_map["endpoint"][$method]; - return $function($method, $data, $query_params); + return $handler($method, $data, $query_params); } - public function add_validator($label, $function) + public function add_validator(string $label, callable $function): void { $this->_validators[$label] = $function; } - private function get_validator($url_map) + /** @return array{0: string, 1: callable} */ + private function get_validator(TrieNode $node): array { - foreach (array_keys($url_map) as $route) { + foreach (array_keys($node->children) as $route) { if (startswith($route, ":")) { return [$route, $this->_validators[$route]]; } @@ -73,8 +93,9 @@ class ApiRouter throw new InvalidAPI(); } - public static function get_router() + public static function get_router(): ApiRouter { + /** @var ?ApiRouter */ static $router = null; if (!$router) { $router = new ApiRouter(); diff --git a/api/v1_projects.inc b/api/v1_projects.inc index 3271a44a2d..e885bf4378 100644 --- a/api/v1_projects.inc +++ b/api/v1_projects.inc @@ -36,7 +36,7 @@ function get_return_fields(?array $query_params, ?Project $project) return $return_fields; } -function api_v1_projects($method, $data, $query_params) +function api_v1_projects(string $method, array $data, array $query_params) { // set which fields are queryable and their column names $valid_fields = get_project_fields_with_attr("queryable", null); @@ -306,7 +306,7 @@ function create_or_update_project(Project $project) return $project; } -function api_v1_project($method, $data, $query_params) +function api_v1_project(string $method, array $data, array $query_params) { if ($method == "GET") { // restrict to list of desired fields, if set @@ -373,7 +373,7 @@ function render_project_json(Project $project, ?array $return_fields = null) //--------------------------------------------------------------------------- // projects/:projectID/wordlists/:type -function api_v1_project_wordlists($method, $data, $query_params) +function api_v1_project_wordlists(string $method, array $data, array $query_params) { // get the project this is for and the type of word list $project = $data[":projectid"]; @@ -406,7 +406,7 @@ function api_v1_project_wordlists($method, $data, $query_params) //--------------------------------------------------------------------------- // projects/:projectID/holdstates -function api_v1_project_holdstates($method, $data, $query_params) +function api_v1_project_holdstates(string $method, array $data, array $query_params) { $project = $data[":projectid"]; @@ -446,7 +446,7 @@ function api_v1_project_holdstates($method, $data, $query_params) //--------------------------------------------------------------------------- // projects/:projectid/pages -function api_v1_project_pages($method, $data, $query_params) +function api_v1_project_pages(string $method, array $data, array $query_params) { $project = $data[":projectid"]; @@ -468,7 +468,7 @@ function api_v1_project_pages($method, $data, $query_params) //--------------------------------------------------------------------------- // projects/:projectid/pagedetails -function api_v1_project_pagedetails($method, $data, $query_params) +function api_v1_project_pagedetails(string $method, array $data, array $query_params) { // optional page round IDs (one or more) to filter down to $only_rounds = null; @@ -479,7 +479,7 @@ function api_v1_project_pagedetails($method, $data, $query_params) $pageroundids = [$pageroundids]; } foreach ($pageroundids as $pageroundid) { - validate_page_round($pageroundid, null); + validate_page_round($pageroundid, []); if ($pageroundid === "OCR") { $only_rounds[] = "OCR"; } else { @@ -524,7 +524,7 @@ function api_v1_project_pagedetails($method, $data, $query_params) //--------------------------------------------------------------------------- // projects/:projectid/pages/:pagename/pagerounds/:pageroundid -function api_v1_project_page_round($method, $data, $query_params) +function api_v1_project_page_round(string $method, array $data, array $query_params) { if ($data[":pageroundid"] == "OCR") { $text_column = "master_text"; @@ -583,7 +583,7 @@ function render_project_page_json($row) //--------------------------------------------------------------------------- // projects/:projectid/transitions -function api_v1_project_transitions($method, $data, $query_params) +function api_v1_project_transitions(string $method, array $data, array $query_params) { $sql = sprintf( " @@ -610,7 +610,7 @@ function api_v1_project_transitions($method, $data, $query_params) //--------------------------------------------------------------------------- // projects/difficulties -function api_v1_projects_difficulties($method, $data, $query_params) +function api_v1_projects_difficulties(string $method, array $data, array $query_params) { $difficulties = get_project_difficulties(); return array_keys($difficulties); @@ -619,7 +619,7 @@ function api_v1_projects_difficulties($method, $data, $query_params) //--------------------------------------------------------------------------- // projects/genres -function api_v1_projects_genres($method, $data, $query_params) +function api_v1_projects_genres(string $method, array $data, array $query_params) { $genres = ProjectSearchForm::genre_options(); unset($genres['']); @@ -629,7 +629,7 @@ function api_v1_projects_genres($method, $data, $query_params) //--------------------------------------------------------------------------- // projects/languages -function api_v1_projects_languages($method, $data, $query_params) +function api_v1_projects_languages(string $method, array $data, array $query_params) { $languages = ProjectSearchForm::language_options(); unset($languages['']); @@ -639,7 +639,7 @@ function api_v1_projects_languages($method, $data, $query_params) //--------------------------------------------------------------------------- // projects/states -function api_v1_projects_states($method, $data, $query_params) +function api_v1_projects_states(string $method, array $data, array $query_params) { $states = ProjectSearchForm::state_options(); unset($states['']); @@ -649,7 +649,7 @@ function api_v1_projects_states($method, $data, $query_params) //--------------------------------------------------------------------------- // projects/pagerounds -function api_v1_projects_pagerounds($method, $data, $query_params) +function api_v1_projects_pagerounds(string $method, array $data, array $query_params) { return array_merge(["OCR"], Rounds::get_ids()); } @@ -657,7 +657,7 @@ function api_v1_projects_pagerounds($method, $data, $query_params) //--------------------------------------------------------------------------- // projects/charsuites -function api_v1_projects_charsuites($method, $data, $query_params) +function api_v1_projects_charsuites(string $method, array $data, array $query_params) { $enabled_filter = _get_enabled_filter($query_params); if ($enabled_filter === null) { @@ -686,7 +686,7 @@ function api_v1_projects_charsuites($method, $data, $query_params) //--------------------------------------------------------------------------- // projects/specialdays -function api_v1_projects_specialdays($method, $data, $query_params) +function api_v1_projects_specialdays(string $method, array $data, array $query_params) { $return_data = []; @@ -720,7 +720,7 @@ function api_v1_projects_specialdays($method, $data, $query_params) //--------------------------------------------------------------------------- // projects/imagesources -function api_v1_projects_imagesources($method, $data, $query_params) +function api_v1_projects_imagesources(string $method, array $data, array $query_params): array { $return_data = []; @@ -755,7 +755,7 @@ function api_v1_projects_imagesources($method, $data, $query_params) //--------------------------------------------------------------------------- // projects/holdstates -function api_v1_projects_holdstates($method, $data, $query_params) +function api_v1_projects_holdstates(string $method, array $data, array $query_params): array { return Project::get_holdable_states(); } @@ -780,7 +780,7 @@ function _get_enabled_filter($query_params) // Proofreading functions // checkout a page -function api_v1_project_checkout($method, array $data, array $query_params) +function api_v1_project_checkout(string $method, array $data, array $query_params): array { try { $project = $data[":projectid"]; @@ -795,14 +795,14 @@ function api_v1_project_checkout($method, array $data, array $query_params) } } -function api_v1_project_validatetext(string $method, array $data, array $query_params) +function api_v1_project_validatetext(string $method, array $data, array $query_params): array { $project = $data[":projectid"]; $invalid_characters = $project->find_invalid_characters(receive_project_text_from_request_body('text')); return ["invalid_chars" => $invalid_characters]; } -function api_v1_project_wordcheck($method, $data, array $query_params) +function api_v1_project_wordcheck(string $method, array $data, array $query_params): array { $project = $data[":projectid"]; $accepted_words = receive_data_from_request_body("accepted_words") ?? []; @@ -832,6 +832,8 @@ function api_v1_project_pickersets(string $method, array $data, array $query_par return $verbose_pickersets; } +// TODO(jchaffraix): Refine this return once all callees have been typed. +/** @return mixed */ function api_v1_project_page(string $method, array $data, array $query_params) { global $pguser; @@ -894,7 +896,7 @@ function api_v1_project_page_wordcheck(string $method, array $data, array $query } } -function validate_project_state($project, $state) +function validate_project_state(Project $project, ?string $state): void { if (null === $state) { throw new InvalidValue("No project state found in request."); @@ -913,7 +915,7 @@ function validate_project_state($project, $state) } } -function validate_page_state($project_page, $page_state) +function validate_page_state(ProjectPage $project_page, ?string $page_state): void { if (null === $page_state) { throw new InvalidValue("No page state found in request."); @@ -941,7 +943,7 @@ function receive_project_text_from_request_body(): string return $page_text; } -function receive_data_from_request_body($field) +function receive_data_from_request_body(string $field) { $request_data = api_get_request_body(); return $request_data[$field] ?? null; diff --git a/api/v1_queues.inc b/api/v1_queues.inc index d616fa7066..d4a989d18c 100644 --- a/api/v1_queues.inc +++ b/api/v1_queues.inc @@ -11,7 +11,7 @@ include_once("api_common.inc"); function api_v1_queues(string $method, array $data, array $query_params): array { $roundid = $query_params["roundid"] ?? null; - $round = !is_null($roundid) ? validate_round($roundid, null) : null; + $round = !is_null($roundid) ? validate_round($roundid, []) : null; $show = get_enumerated_param($query_params, "show", "all", ["enabled", "populated", "all"]); @@ -68,7 +68,7 @@ function api_v1_queue_stats(string $method, array $data, array $query_params): a { $queue = $data[":queueid"]; - $round = validate_round($queue["round_id"], null); + $round = validate_round($queue["round_id"], []); $name = $queue["name"]; return fetch_queue_stats_data($round, $name); @@ -96,7 +96,7 @@ function api_v1_queue_projects(string $method, array $data, array $query_params) $return_fields = array_get_as_array($query_params, "field", $valid_return_fields); $return_fields = array_intersect($return_fields, $valid_return_fields); - $round = validate_round($queue["round_id"], null); + $round = validate_round($queue["round_id"], []); $output = []; foreach (fetch_queue_projects_data($round, $queue["project_selector"], $unheld_only) as $proj_data) { diff --git a/api/v1_validators.inc b/api/v1_validators.inc index ba18ca5a6b..a54656c100 100644 --- a/api/v1_validators.inc +++ b/api/v1_validators.inc @@ -5,7 +5,7 @@ include_once($relPath.'release_queue.inc'); //=========================================================================== // Validators -function validate_round($roundid, $data) +function validate_round(string $roundid, array $_data): Round { try { $round = Rounds::get_by_id($roundid); @@ -18,7 +18,7 @@ function validate_round($roundid, $data) } } -function validate_project($projectid, $data) +function validate_project(string $projectid, array $_data): Project { // validate and load the specified projectid try { @@ -28,7 +28,7 @@ function validate_project($projectid, $data) } } -function validate_wordlist($wordlist, $data) +function validate_wordlist(array $wordlist, array $_data): array { if (!in_array($wordlist, ["good", "bad"])) { throw new NotFoundError(); @@ -36,7 +36,7 @@ function validate_wordlist($wordlist, $data) return $wordlist; } -function validate_page_name($pagename, $data) +function validate_page_name(string $pagename, array $data): ProjectPage { try { return $data[":projectid"]->get_project_page($pagename); @@ -45,7 +45,7 @@ function validate_page_name($pagename, $data) } } -function validate_page_round($pageround, $data) +function validate_page_round(string $pageround, array $_data): string { try { $pagerounds = array_merge(["OCR"], Rounds::get_ids()); diff --git a/pinc/Project.inc b/pinc/Project.inc index 8652c75704..cbd4cd1b9b 100644 --- a/pinc/Project.inc +++ b/pinc/Project.inc @@ -1525,7 +1525,7 @@ class Project DPDatabase::query($sql); } - public static function get_holdable_states() + public static function get_holdable_states(): array { $states = []; foreach (Rounds::get_all() as $round) { @@ -1792,7 +1792,7 @@ class Project } } - public function get_project_page($pagename) + public function get_project_page(?string $pagename): ProjectPage { if (!$this->pages_table_exists) { throw new NoProjectPageTable(_("Project page table does not exist."));