From 0acc756f5bdf4560e3d0106a30868fa70b61ed84 Mon Sep 17 00:00:00 2001 From: Julien Chaffraix Date: Wed, 8 Jan 2025 22:11:35 -0800 Subject: [PATCH] Type api/ApiRouter.inc This PR reworks the ApiRouter to clearly state that it was using a trie and add an appropriate TrieNode. This helped clarify what the code does and clean up the types in the class. TESTS=Tried calling some APIs after the change to confirm that the routing still works. --- SETUP/tests/unittests/ApiTest.php | 12 +++--- api/ApiRouter.inc | 69 ++++++++++++++++++++----------- 2 files changed, 51 insertions(+), 30 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();