From d70d916936e032a21561f23dce19eebd08ebc35f Mon Sep 17 00:00:00 2001 From: Dan Feder Date: Fri, 28 Jun 2024 18:04:32 -0400 Subject: [PATCH] Fix broken reference in OpenAPI spec, improve validation tests (#4201) --- .circleci/config.yml | 5 -- composer.json | 1 + .../tests/src/Functional/Api1TestBase.php | 9 ++++ modules/metastore/docs/openapi_spec.json | 51 ++++++++++++++----- .../Plugin/DkanApiDocs/MetastoreApiDocs.php | 19 +++---- .../src/Functional/Api1/DatasetItemTest.php | 33 ++++-------- .../Functional/Api1/DatasetRevisionTest.php | 8 +-- .../Api1/DistributionHandlingTest.php | 2 - 8 files changed, 72 insertions(+), 56 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 2ebd12a80f..9a03ad72d5 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -248,11 +248,6 @@ workflows: parameters: dkan_recommended_branch: [ '10.1.x-dev'] php_version: [ '8.2' ] - - phpunit: - matrix: - parameters: - dkan_recommended_branch: [ '10.0.x-dev'] - php_version: [ '8.1' ] upgrade_and_test: jobs: - cypress: diff --git a/composer.json b/composer.json index 6f0c844bd3..c7beadddf0 100644 --- a/composer.json +++ b/composer.json @@ -40,6 +40,7 @@ "require-dev": { "drush/drush": "^12@stable", "getdkan/mock-chain": "^1.3.7", + "osteel/openapi-httpfoundation-testing": "<1.0", "phpunit/phpunit": "^8.5.14 || ^9", "weitzman/drupal-test-traits": "^2.0.1" }, diff --git a/modules/common/tests/src/Functional/Api1TestBase.php b/modules/common/tests/src/Functional/Api1TestBase.php index 3b04e5694f..ee8cf53598 100644 --- a/modules/common/tests/src/Functional/Api1TestBase.php +++ b/modules/common/tests/src/Functional/Api1TestBase.php @@ -8,6 +8,7 @@ use GuzzleHttp\RequestOptions; use Opis\JsonSchema\Schema; use Opis\JsonSchema\Validator; +use Osteel\OpenApi\Testing\ValidatorBuilder; abstract class Api1TestBase extends BrowserTestBase { use UserCreationTrait; @@ -23,6 +24,13 @@ abstract class Api1TestBase extends BrowserTestBase { protected $auth; protected $endpoint; + /** + * OpenApi Validator. + * + * @var \Osteel\OpenApi\Testing\ValidatorInterface + */ + protected $validator; + protected $defaultTheme = 'stark'; protected $strictConfigSchema = FALSE; @@ -49,6 +57,7 @@ public function setUp(): void { // Load the API spec for use by tests. $response = $this->httpClient->request('GET', 'api/1'); + $this->validator = ValidatorBuilder::fromJsonString($response->getBody())->getValidator(); $this->spec = json_decode($response->getBody()); } diff --git a/modules/metastore/docs/openapi_spec.json b/modules/metastore/docs/openapi_spec.json index 285ed1c131..aaf74e1014 100644 --- a/modules/metastore/docs/openapi_spec.json +++ b/modules/metastore/docs/openapi_spec.json @@ -25,22 +25,35 @@ } }, "responses": { + "200MetadataUpdated": { + "description": "Metadata update successful.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/metastoreWriteResponse" + } + } + } + }, "201MetadataCreated": { "description": "Metadata creation successful.", "content": { "application/json": { "schema": { - "type": "object", - "properties": { - "endpoint": { - "type": "string", - "description": "Path to the metadata from the API." - }, - "identifier": { - "type": "string", - "description": "Identifier for metadata just created or modified." - } - } + "$ref": "#/components/schemas/metastoreWriteResponse" + } + } + } + }, + "409MetadataAlreadyExists": { + "description": "Conflict; tried to create a record using an existing ID, or metadata contains identifier that doesn't match the request path.", + "content": { + "application/json": { + "schema": { "$ref": "#/components/schemas/errorResponse" }, + "example": { + "message": "dataset/00000000-0000-0000-0000-000000000000 already exists.", + "status": 409, + "timestamp": "2021-06-14T13:46:06+00:00" } } } @@ -61,6 +74,20 @@ }, "schemas": { + "metastoreWriteResponse": { + "type": "object", + "properties": { + "endpoint": { + "type": "string", + "description": "Path to the metadata from the API." + }, + "identifier": { + "type": "string", + "description": "Identifier for metadata just created or modified." + } + }, + "additionalProperties": false + }, "metastoreRevision": { "type": "object", "properties": { @@ -181,7 +208,7 @@ ], "responses": { "200": { - "description": "Ok", + "description": "Full list of all items for the given schema", "content": { "application/json": { "schema": { diff --git a/modules/metastore/src/Plugin/DkanApiDocs/MetastoreApiDocs.php b/modules/metastore/src/Plugin/DkanApiDocs/MetastoreApiDocs.php index ae730949e8..92d70da9ae 100644 --- a/modules/metastore/src/Plugin/DkanApiDocs/MetastoreApiDocs.php +++ b/modules/metastore/src/Plugin/DkanApiDocs/MetastoreApiDocs.php @@ -49,7 +49,7 @@ public function __construct( $pluginDefinition, ModuleHandlerInterface $moduleHandler, TranslationInterface $stringTranslation, - MetastoreService $metastore + MetastoreService $metastore, ) { parent::__construct($configuration, $pluginId, $pluginDefinition, $moduleHandler, $stringTranslation); $this->metastore = $metastore; @@ -73,7 +73,7 @@ public static function create( ContainerInterface $container, array $configuration, $pluginId, - $pluginDefinition + $pluginDefinition, ) { return new static( $configuration, @@ -296,6 +296,7 @@ private function schemaItemPost($schemaId, array $schema) { ], ], '400' => ['$ref' => '#/components/responses/400BadJson'], + '409' => ['$ref' => '#/components/responses/409MetadataAlreadyExists'], ], ]; } @@ -347,7 +348,7 @@ private function schemaItemPut($schemaId) { return [ "operationId" => "$schemaId-put", "summary" => $this->t("Replace a :schemaId", $tSchema), - "description" => $this->t("Object will be completely replaced; optional properties not included in the request will be deleted.\n\nAutomatic example not yet available; try retrieving a :schemaId via GET, changing values, and pasting to test.", $tSchema), + "description" => $this->t("Object will be completely replaced; optional properties not included in the request will be deleted.\n\nAutomatic example not yet available; try retrieving a :schemaId via GET, changing values, and pasting to test. If no item exists with the provided identifier, it will be created.", $tSchema), "tags" => [$this->t("Metastore: :schemaId", $tSchema)], "security" => [ ['basic_auth' => []], @@ -362,9 +363,10 @@ private function schemaItemPut($schemaId) { ], ], "responses" => [ - "200" => [ - "description" => "Ok.", - ], + "200" => ['$ref' => '#/components/responses/200MetadataUpdated'], + "201" => ['$ref' => '#/components/responses/201MetadataCreated'], + '400' => ['$ref' => '#/components/responses/400BadJson'], + '409' => ['$ref' => '#/components/responses/409MetadataAlreadyExists'], "412" => ['$ref' => '#/components/responses/412MetadataObjectNotFound'], ], ]; @@ -401,9 +403,8 @@ private function schemaItemPatch($schemaId, array $schema) { ], ], "responses" => [ - "200" => [ - "description" => "Ok.", - ], + "200" => ['$ref' => '#/components/responses/200MetadataUpdated'], + '400' => ['$ref' => '#/components/responses/400BadJson'], "412" => ['$ref' => '#/components/responses/412MetadataObjectNotFound'], ], ]; diff --git a/modules/metastore/tests/src/Functional/Api1/DatasetItemTest.php b/modules/metastore/tests/src/Functional/Api1/DatasetItemTest.php index 652792ef5a..47060714e9 100644 --- a/modules/metastore/tests/src/Functional/Api1/DatasetItemTest.php +++ b/modules/metastore/tests/src/Functional/Api1/DatasetItemTest.php @@ -19,13 +19,13 @@ public function testGet() { $this->post($this->getSampleDataset(1)); - $responseSchema = $this->spec->paths->{'/api/1/metastore/schemas/{schema_id}/items'} - ->get->responses->{"200"}->content->{"application/json"}->schema; $response = $this->httpClient->request('GET', $this->endpoint); $responseBody = json_decode($response->getBody()); $this->assertEquals(2, count($responseBody)); $this->assertTrue(is_object($responseBody[1])); - $this->assertJsonIsValid($responseSchema, $responseBody); + // Have to use this path because the endpoint as added is not in the spec. + // @todo Simplify dataset vs {schema_id} items in the spec. + $this->validator->validate($response, "api/1/metastore/schemas/{schema_id}/items", 'get'); $datasetId = 'abc-123'; $response = $this->httpClient->get("$this->endpoint/$datasetId", [ @@ -33,9 +33,7 @@ public function testGet() { ]); $this->assertEquals(404, $response->getStatusCode()); - $responseBody = json_decode($response->getBody()); - $responseSchema = $this->spec->components->responses->{"404IdNotFound"}; - $this->assertJsonIsValid($responseSchema, $responseBody); + $this->validator->validate($response, "$this->endpoint/$datasetId", 'get'); } public function testPost() { @@ -43,16 +41,13 @@ public function testPost() { $response = $this->post($dataset); $this->assertEquals(201, $response->getStatusCode()); - $responseBody = json_decode($response->getBody()); - $responseSchema = $this->spec->components->responses->{"201MetadataCreated"}->content->{"application/json"}->schema; - - $this->assertJsonIsValid($responseSchema, $responseBody); + $this->validator->validate($response, $this->endpoint, 'post'); $this->assertDatasetGet($dataset); // Now try a duplicate. $response = $this->post($dataset, FALSE); $this->assertEquals(409, $response->getStatusCode()); - // @todo Fuly validate response once documented. + $this->validator->validate($response, $this->endpoint, 'post'); } public function testPatch() { @@ -68,9 +63,7 @@ public function testPatch() { $this->assertEquals(200, $response->getStatusCode()); - $responseBody = json_decode($response->getBody()); - $responseSchema = $this->spec->components->responses->{"201MetadataCreated"}->content->{"application/json"}->schema; - $this->assertJsonIsValid($responseSchema, $responseBody); + $this->validator->validate($response, "$this->endpoint/$datasetId", 'patch'); $dataset->title = $newTitle->title; $this->assertDatasetGet($dataset); @@ -86,10 +79,8 @@ public function testPatch() { ]); $this->assertEquals(412, $response->getStatusCode()); + $this->validator->validate($response, "$this->endpoint/$datasetId", 'patch'); - $responseBody = json_decode($response->getBody()); - $responseSchema = $this->spec->components->responses->{"412MetadataObjectNotFound"}; - $this->assertJsonIsValid($responseSchema, $responseBody); } public function testPut() { @@ -105,9 +96,7 @@ public function testPut() { RequestOptions::AUTH => $this->auth, ]); $this->assertEquals(200, $response->getStatusCode()); - $responseBody = json_decode($response->getBody()); - $responseSchema = $this->spec->components->responses->{"201MetadataCreated"}->content->{"application/json"}->schema; - $this->assertJsonIsValid($responseSchema, $responseBody); + $this->validator->validate($response, "$this->endpoint/$datasetId", 'put'); $this->assertDatasetGet($newDataset); // Now try with mismatched identifiers. @@ -118,15 +107,15 @@ public function testPut() { RequestOptions::HTTP_ERRORS => FALSE, ]); $this->assertEquals(409, $response->getStatusCode()); + $this->validator->validate($response, "$this->endpoint/$datasetId", 'put'); } private function assertDatasetGet($dataset) { $id = $dataset->identifier; - $responseSchema = $this->spec->components->schemas->dataset; $response = $this->httpClient->get("$this->endpoint/$id"); $responseBody = json_decode($response->getBody()); $this->assertEquals(200, $response->getStatusCode()); - $this->assertJsonIsValid($responseSchema, $responseBody); + $this->validator->validate($response, "$this->endpoint/$id", 'get'); $this->assertEquals($dataset, $responseBody); } diff --git a/modules/metastore/tests/src/Functional/Api1/DatasetRevisionTest.php b/modules/metastore/tests/src/Functional/Api1/DatasetRevisionTest.php index 05a3ea9c7f..f5082e21b6 100644 --- a/modules/metastore/tests/src/Functional/Api1/DatasetRevisionTest.php +++ b/modules/metastore/tests/src/Functional/Api1/DatasetRevisionTest.php @@ -20,10 +20,7 @@ public function testList() { ]); // Test individual item endpoint. - $responseSchema = $this->spec->components->responses->{"201MetadataCreated"}->content->{"application/json"}->schema; - - $responseBody = json_decode($response->getBody()); - $this->assertJsonIsValid($responseSchema, $responseBody); + $this->validator->validate($response, "api/1/metastore/schemas/dataset/items", 'post'); $response = $this->httpClient->get($this->endpoint, [ RequestOptions::AUTH => $this->auth, @@ -115,7 +112,6 @@ public function testPost() { 'archived' => FALSE, 'hidden' => TRUE, ]; - $responseSchema = $this->spec->components->responses->{"201MetadataCreated"}->content->{"application/json"}->schema; $count = 1; foreach ($states as $state => $public) { @@ -126,7 +122,7 @@ public function testPost() { // Validate response object. $responseBody = json_decode($response->getBody()); - $this->assertJsonIsValid($responseSchema, $responseBody); + $this->validator->validate($response, $this->endpoint, 'post'); // Validate URL and contents of response object. $response = $this->httpClient->get($responseBody->endpoint, [ diff --git a/modules/metastore/tests/src/Functional/Api1/DistributionHandlingTest.php b/modules/metastore/tests/src/Functional/Api1/DistributionHandlingTest.php index 2868ef64ff..aa4612fa40 100644 --- a/modules/metastore/tests/src/Functional/Api1/DistributionHandlingTest.php +++ b/modules/metastore/tests/src/Functional/Api1/DistributionHandlingTest.php @@ -95,9 +95,7 @@ private function postDataDictionary() { $this->assertEquals(201, $response->getStatusCode()); $responseBody = json_decode($response->getBody()); - $responseSchema = $this->spec->components->responses->{"201MetadataCreated"}->content->{"application/json"}->schema; - $this->assertJsonIsValid($responseSchema, $responseBody); // Unless JSON changes, we should always get same id back. $this->assertEquals("47f1d697-f469-5b41-a613-80cdfac7a326", $responseBody->identifier);