Fix broken reference in OpenAPI spec, improve validation tests (#4201)

GetDKAN · Jun 28, 2024 · d70d916 · d70d916
1 parent db9ceb6
commit d70d916
Show file tree

Hide file tree

Showing 8 changed files with 72 additions and 56 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -19,40 +19,35 @@ 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", [
       RequestOptions::HTTP_ERRORS => FALSE,
     ]);
     $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() {
     $dataset = $this->getSampleDataset();
     $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
@@ -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
@@ -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);