diff --git a/CHANGELOG.md b/CHANGELOG.md index a363d709ab9..80527b8c1a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ The present file will list all changes made to the project; according to the ### Added ### Changed +- Added High-Level API version 2.1. Make sure you are pinning your requests to a specific version (Ex: `/api.php/v2.0`) if needed to exclude endpoints/properties added in later versions. See version pinning in the getting started documentation `/api.php/getting-started`. +- High-Level API responses for not found routes now correctly return a body including the standard error properties (status, title, detail). This is not controlled by the API version. ### Deprecated diff --git a/src/Glpi/Api/HL/Controller/CoreController.php b/src/Glpi/Api/HL/Controller/CoreController.php index 6b93c7d3e9a..0a12255653e 100644 --- a/src/Glpi/Api/HL/Controller/CoreController.php +++ b/src/Glpi/Api/HL/Controller/CoreController.php @@ -163,7 +163,8 @@ public function showDocumentation(Request $request): Response $swagger_content .= Html::script('/lib/swagger-ui.js'); $swagger_content .= Html::css('/lib/swagger-ui.css'); $favicon = Html::getPrefixedUrl('/pics/favicon.ico'); - $doc_json_path = $CFG_GLPI['root_doc'] . '/api.php/doc.json'; + $api_version = $this->getAPIVersion($request); + $doc_json_path = $CFG_GLPI['root_doc'] . '/api.php/v' . $api_version . '/doc.json'; $swagger_content .= << @@ -285,7 +286,7 @@ private function getAllowedMethodsForMatchedRoute(Request $request): array )] public function defaultRoute(Request $request): Response { - return new JSONResponse(null, 404); + return self::getNotFoundErrorResponse(); } #[Route(path: '/{req}', methods: ['OPTIONS'], requirements: ['req' => '.*'], priority: -1, security_level: Route::SECURITY_NONE)] @@ -299,7 +300,7 @@ public function defaultOptionsRoute(Request $request): Response $authenticated = Session::getLoginUserID() !== false; $allowed_methods = $authenticated ? $this->getAllowedMethodsForMatchedRoute($request) : ['GET', 'POST', 'PATCH', 'PUT', "DELETE"]; if (count($allowed_methods) === 0) { - return new JSONResponse(null, 404); + return self::getNotFoundErrorResponse(); } $response_headers = []; if ($authenticated) { diff --git a/src/Glpi/Api/HL/Doc/Schema.php b/src/Glpi/Api/HL/Doc/Schema.php index 59eca68c866..d191484d190 100644 --- a/src/Glpi/Api/HL/Doc/Schema.php +++ b/src/Glpi/Api/HL/Doc/Schema.php @@ -337,6 +337,8 @@ public static function filterSchemaByAPIVersion(array $schema, string $api_versi 'removed' => $schema['x-version-removed'] ?? null, ]; + $api_version = Router::normalizeAPIVersion($api_version); + // Check if the schema itself is applicable to the requested version // If the requested version is before the introduction of the schema, or after the removal of the schema, it is not applicable // Deprecation has no effect here diff --git a/src/Glpi/Api/HL/OpenAPIGenerator.php b/src/Glpi/Api/HL/OpenAPIGenerator.php index 4c6539a56f9..a4f26f98819 100644 --- a/src/Glpi/Api/HL/OpenAPIGenerator.php +++ b/src/Glpi/Api/HL/OpenAPIGenerator.php @@ -131,7 +131,7 @@ private function getInfo(): array return [ 'title' => 'GLPI High-Level REST API', 'description' => $description, - 'version' => Router::API_VERSION, + 'version' => $this->api_version, 'license' => [ 'name' => 'GNU General Public License v3 or later', 'url' => 'https://www.gnu.org/licenses/gpl-3.0.html', @@ -141,40 +141,36 @@ private function getInfo(): array public static function getComponentSchemas(string $api_version): array { - static $schemas = null; - - if ($schemas === null) { - $schemas = []; - - $controllers = Router::getInstance()->getControllers(); - foreach ($controllers as $controller) { - $known_schemas = $controller::getKnownSchemas($api_version); - $short_name = (new ReflectionClass($controller))->getShortName(); - $controller_name = str_replace('Controller', '', $short_name); - foreach ($known_schemas as $schema_name => $known_schema) { - // Ignore schemas starting with an underscore. They are only used internally. - if (str_starts_with($schema_name, '_')) { - continue; - } - $calculated_name = $schema_name; - if (isset($schemas[$schema_name])) { - // For now, set the new calculated name to the short name of the controller + the schema name - $calculated_name = $controller_name . ' - ' . $schema_name; - // Change the existing schema name to its own calculated name - $other_short_name = (new ReflectionClass($schemas[$schema_name]['x-controller']))->getShortName(); - $other_calculated_name = str_replace('Controller', '', $other_short_name) . ' - ' . $schema_name; - $schemas[$other_calculated_name] = $schemas[$schema_name]; - unset($schemas[$schema_name]); - } - if (!isset($known_schema['description']) && isset($known_schema['x-itemtype'])) { - /** @var class-string $itemtype */ - $itemtype = $known_schema['x-itemtype']; - $known_schema['description'] = $itemtype::getTypeName(1); - } - $schemas[$calculated_name] = $known_schema; - $schemas[$calculated_name]['x-controller'] = $controller::class; - $schemas[$calculated_name]['x-schemaname'] = $schema_name; + $schemas = []; + + $controllers = Router::getInstance()->getControllers(); + foreach ($controllers as $controller) { + $known_schemas = $controller::getKnownSchemas($api_version); + $short_name = (new ReflectionClass($controller))->getShortName(); + $controller_name = str_replace('Controller', '', $short_name); + foreach ($known_schemas as $schema_name => $known_schema) { + // Ignore schemas starting with an underscore. They are only used internally. + if (str_starts_with($schema_name, '_')) { + continue; } + $calculated_name = $schema_name; + if (isset($schemas[$schema_name])) { + // For now, set the new calculated name to the short name of the controller + the schema name + $calculated_name = $controller_name . ' - ' . $schema_name; + // Change the existing schema name to its own calculated name + $other_short_name = (new ReflectionClass($schemas[$schema_name]['x-controller']))->getShortName(); + $other_calculated_name = str_replace('Controller', '', $other_short_name) . ' - ' . $schema_name; + $schemas[$other_calculated_name] = $schemas[$schema_name]; + unset($schemas[$schema_name]); + } + if (!isset($known_schema['description']) && isset($known_schema['x-itemtype'])) { + /** @var class-string $itemtype */ + $itemtype = $known_schema['x-itemtype']; + $known_schema['description'] = $itemtype::getTypeName(1); + } + $schemas[$calculated_name] = $known_schema; + $schemas[$calculated_name]['x-controller'] = $controller::class; + $schemas[$calculated_name]['x-schemaname'] = $schema_name; } } @@ -255,6 +251,9 @@ public function getSchema(): array $paths = []; foreach ($routes as $route_path) { + if (!$route_path->matchesAPIVersion($this->api_version)) { + continue; + } /** @noinspection SlowArrayOperationsInLoopInspection */ $paths = array_merge_recursive($paths, $this->getPathSchemas($route_path)); } diff --git a/src/Glpi/Api/HL/RoutePath.php b/src/Glpi/Api/HL/RoutePath.php index 29cf1c00e24..abf3b368848 100644 --- a/src/Glpi/Api/HL/RoutePath.php +++ b/src/Glpi/Api/HL/RoutePath.php @@ -362,7 +362,10 @@ public function getRouteVersion(): RouteVersion public function matchesAPIVersion(string $api_version): bool { $version = $this->getRouteVersion(); - return (version_compare($api_version, $version->introduced, '>=') && (empty($version->removed) || version_compare($api_version, $version->removed, '<'))); + return ( + version_compare($api_version, $version->introduced, '>=') + && (empty($version->removed) || version_compare($api_version, $version->removed, '<')) + ); } private function setPath(string $path) diff --git a/src/Glpi/Api/HL/Router.php b/src/Glpi/Api/HL/Router.php index 2a201f9b47c..4e1c1d71df6 100644 --- a/src/Glpi/Api/HL/Router.php +++ b/src/Glpi/Api/HL/Router.php @@ -89,7 +89,7 @@ class Router { /** @var string */ - public const API_VERSION = '2.0.0'; + public const API_VERSION = '2.1.0'; /** * @var AbstractController[] @@ -152,10 +152,6 @@ public static function getAPIVersions(): array While not as user friendly as the high-level API, it is more powerful and allows to do some things that are not possible with the high-level API. It has no promise of stability between versions so it may change without warning. EOT; - $current_version = self::API_VERSION; - // Get short version which is the major part of the semver string - $current_version_major = explode('.', $current_version)[0]; - return [ [ 'api_version' => '1', @@ -164,9 +160,14 @@ public static function getAPIVersions(): array 'endpoint' => $CFG_GLPI['url_base'] . '/api.php/v1', ], [ - 'api_version' => $current_version_major, - 'version' => self::API_VERSION, - 'endpoint' => $CFG_GLPI['url_base'] . '/api.php/v2', + 'api_version' => '2', + 'version' => '2.0.0', + 'endpoint' => $CFG_GLPI['url_base'] . '/api.php/v2.0', + ], + [ + 'api_version' => '2', + 'version' => '2.1.0', + 'endpoint' => $CFG_GLPI['url_base'] . '/api.php/v2.1', ], ]; } @@ -181,20 +182,24 @@ public static function getAPIVersions(): array * @param string $version * @return string */ - public static function normalizeAPIVersion(string $version): string + final public static function normalizeAPIVersion(string $version): string { $versions = array_column(static::getAPIVersions(), 'version'); - $best_match = self::API_VERSION; - if (in_array($version, $versions, true)) { - // Exact match - return $version; + $best_match = null; + if (empty($version)) { + $version = static::API_VERSION; } foreach ($versions as $available_version) { - if (str_starts_with($available_version, $version . '.') && version_compare($available_version, $best_match, '>')) { - $best_match = $available_version; + if (str_starts_with($available_version, $version)) { + if ($best_match === null || version_compare($available_version, $best_match, '>')) { + $best_match = $available_version; + } } } + if ($best_match === null) { + $best_match = static::API_VERSION; + } return $best_match; } @@ -488,7 +493,7 @@ public function matchAll(Request $request): array { $routes = $this->getRoutesFromCache(); - $api_version = $request->getHeaderLine('GLPI-API-Version') ?: static::API_VERSION; + $api_version = self::normalizeAPIVersion($request->getHeaderLine('GLPI-API-Version') ?: static::API_VERSION); // Filter routes by the requested API version and method $routes = array_filter($routes, static function ($route) use ($request, $api_version) { if ($route->matchesAPIVersion($api_version) && in_array($request->getMethod(), $route->getRouteMethods(), true)) { diff --git a/tests/functional/Glpi/Api/HL/RouterTest.php b/tests/functional/Glpi/Api/HL/RouterTest.php index 2f7c29100d2..880a9069442 100644 --- a/tests/functional/Glpi/Api/HL/RouterTest.php +++ b/tests/functional/Glpi/Api/HL/RouterTest.php @@ -87,20 +87,12 @@ public function testAllSchemasHaveVersioningInfo() $this->assertEmpty($schemas_missing_versions, 'Schemas missing versioning info: ' . implode(', ', $schemas_missing_versions)); } - public function testNormalizeAPIVersion() - { - $this->assertEquals('50.2.0', TestRouter::normalizeAPIVersion('50')); - $this->assertEquals('50.1.1', TestRouter::normalizeAPIVersion('50.1.1')); - $this->assertEquals('50.1.2', TestRouter::normalizeAPIVersion('50.1')); - $this->assertEquals('50.2.0', TestRouter::normalizeAPIVersion('50.2')); - } - public function testHLAPIDisabled() { global $CFG_GLPI; $CFG_GLPI['enable_hlapi'] = 0; - $router = TestRouter::getInstance(); + $router = Router::getInstance(); $response = $router->handleRequest(new Request('GET', '/Computer')); $this->assertEquals(403, $response->getStatusCode()); $this->assertStringContainsString('The High-Level API is disabled', (string) $response->getBody()); @@ -110,11 +102,71 @@ public function testHLAPIDisabled() $this->assertEquals(403, $response->getStatusCode()); $this->assertStringContainsString('The High-Level API is disabled', (string) $response->getBody()); } + + public function testNormalizeVersion() + { + // invalid version = router default + $this->assertEquals('51.0.0', TestRouter::normalizeAPIVersion('99')); + // only major version = latest API version for this major + $this->assertEquals('50.2.0', TestRouter::normalizeAPIVersion('50')); + // major.minor version = latest API version for this major.minor + $this->assertEquals('50.1.2', TestRouter::normalizeAPIVersion('50.1')); + // major.minor.patch version = same version + $this->assertEquals('50.1.1', TestRouter::normalizeAPIVersion('50.1.1')); + + $this->assertEquals('50.2.0', TestRouter::normalizeAPIVersion('50.2')); + } + + public function testRoutingByVersion() + { + $router = TestRouter::getInstance(); + // 50.0 is requesting 50.0.X or earlier + $this->assertNotEquals('/{req}', $router->match(new Request('GET', '/version500', ['GLPI-API-Version' => '50.0']))->getRoutePath()); + // 50 is requesting 50.X.X or earlier + $this->assertNotEquals('/{req}', $router->match(new Request('GET', '/version500', ['GLPI-API-Version' => '50']))->getRoutePath()); + // 50.1 is requesting 50.1.X or earlier + $this->assertNotEquals('/{req}', $router->match(new Request('GET', '/version500', ['GLPI-API-Version' => '50.1']))->getRoutePath()); + $this->assertNotEquals('/{req}', $router->match(new Request('GET', '/version500', ['GLPI-API-Version' => '51']))->getRoutePath()); + + $this->assertEquals('/{req}', $router->match(new Request('GET', '/version501', ['GLPI-API-Version' => '50.0']))->getRoutePath()); + $this->assertNotEquals('/{req}', $router->match(new Request('GET', '/version501', ['GLPI-API-Version' => '50.1']))->getRoutePath()); + $this->assertNotEquals('/{req}', $router->match(new Request('GET', '/version501', ['GLPI-API-Version' => '50']))->getRoutePath()); + + $this->assertEquals('/{req}', $router->match(new Request('GET', '/version510', ['GLPI-API-Version' => '50.0']))->getRoutePath()); + $this->assertEquals('/{req}', $router->match(new Request('GET', '/version510', ['GLPI-API-Version' => '50.1']))->getRoutePath()); + $this->assertNotEquals('/{req}', $router->match(new Request('GET', '/version510', ['GLPI-API-Version' => '51']))->getRoutePath()); + $this->assertEquals('/{req}', $router->match(new Request('GET', '/version510', ['GLPI-API-Version' => '50']))->getRoutePath()); + } + + public function testSchemaByVersion() + { + // Note that schema version matching is always done against the "Router" class so it cannot be mocked with the TestRouter versions + $this->assertEquals(['Schema200', 'Schema200_2', 'Schema210'], array_keys(TestController::getKnownSchemas('2'))); + $this->assertEquals(['Schema200', 'Schema200_2'], array_keys(TestController::getKnownSchemas('2.0'))); + $this->assertEquals(['Schema200', 'Schema200_2'], array_keys(TestController::getKnownSchemas('2.0.0'))); + $this->assertEquals(['Schema200', 'Schema200_2', 'Schema210'], array_keys(TestController::getKnownSchemas('2.1'))); + $this->assertEquals(['Schema200', 'Schema200_2', 'Schema210'], array_keys(TestController::getKnownSchemas('2.1.0'))); + + // Test the filtering of fields inside schemas + $schema = TestController::getKnownSchemas('2')['Schema200']; + $this->assertArrayHasKey('field1', $schema['properties']); + $this->assertArrayHasKey('field2', $schema['properties']); + + $schema = TestController::getKnownSchemas('2.0')['Schema200']; + $this->assertArrayHasKey('field1', $schema['properties']); + $this->assertArrayNotHasKey('field2', $schema['properties']); + + $schema = TestController::getKnownSchemas('2.1')['Schema200']; + $this->assertArrayHasKey('field1', $schema['properties']); + $this->assertArrayHasKey('field2', $schema['properties']); + } } // @codingStandardsIgnoreStart class TestRouter extends Router { + public const API_VERSION = '51.0.0'; + // @codingStandardsIgnoreEnd public static function getInstance(): Router { @@ -172,14 +224,74 @@ public static function getAPIVersions(): array class TestController extends AbstractController { // @codingStandardsIgnoreEnd - /** - * @param RequestInterface $request - * @return Response - */ + + protected static function getRawKnownSchemas(): array + { + return [ + 'Schema200' => [ + 'type' => 'object', + 'x-version-introduced' => '2.0', + 'properties' => [ + 'field1' => [ + 'type' => 'string', + ], + 'field2' => [ + 'type' => 'string', + 'x-version-introduced' => '2.1.0', + ], + ], + ], + 'Schema200_2' => [ + 'type' => 'object', + 'x-version-introduced' => '2.0.0', + 'properties' => [ + 'field1' => [ + 'type' => 'string', + ], + + 'field2' => [ + 'type' => 'string', + 'x-version-introduced' => '2.1.0', + ], + ], + ], + 'Schema210' => [ + 'type' => 'object', + 'x-version-introduced' => '2.1.0', + 'properties' => [ + 'field1' => [ + 'type' => 'string', + ], + ], + ], + ]; + } + #[Route('/{req}', ['GET', 'POST', 'PATCH', 'PUT', 'DELETE', 'OPTIONS'], ['req' => '.*'], -1)] - #[RouteVersion(introduced: TestRouter::API_VERSION)] + #[RouteVersion(introduced: '50.0.0')] public function defaultRoute(RequestInterface $request): Response { return new Response(200, [], __FUNCTION__); } + + #[Route('/version500', ['GET'])] + #[RouteVersion(introduced: '50.0.0')] + public function testVersion500(RequestInterface $request): Response + { + return new Response(200, [], __FUNCTION__); + } + + #[Route('/version501', ['GET'])] + #[RouteVersion(introduced: '50.1.0')] + public function testVersion501(RequestInterface $request): Response + { + return new Response(200, [], __FUNCTION__); + } + + #[Route('/version510', ['GET'])] + #[RouteVersion(introduced: '51.0.0')] + public function testVersion510(RequestInterface $request): Response + { + return new Response(200, [], __FUNCTION__); + } }