Refactor web service api docs (#281)

* Break apart main function in dataset-specific docs endpoint into smaller ones
* Unit test incorrectly passed a second argument to getDatasetSpecific()
* Add an sql endpoint to test yml spec to increase code coverage

modules/custom/dkan_metastore/src/WebServiceApiDocs.php

@@ -75,29 +75,58 @@ public function __construct(Docs $docsController, Service $metastoreService) {
    */
   public function getDatasetSpecific(string $identifier) {
     $fullSpec = $this->docsController->getJsonFromYmlFile();
-
     $spec = $this->keepDatasetSpecificEndpoints($fullSpec, $this->endpointsToKeep);
-
     // Remove the security schemes.
     unset($spec['components']['securitySchemes']);
-    // Remove required parameters, since now part of path.
-    unset($spec['paths']['/api/1/datastore/sql']['get']['parameters']);
-    unset($spec['paths']['/api/1/metastore/schemas/dataset/items/{identifier}']['get']['parameters']);
-    // Keep only the tags needed, so remove the properties tag.
-    $spec['tags'] = [
-      ["name" => "Dataset"],
-      ["name" => "SQL Query"],
-    ];
-    // Replace the dataset uuid placeholder.
+    // Tags can be added later when needed, remove them for now.
+    $spec['tags'] = [];
+
+    $spec = $this->modifyDatasetEndpoint($spec, $identifier);
+    $spec = $this->modifySqlEndpoint($spec, $identifier);
+    return $this->getResponse($spec);
+  }
+
+  /**
+   * Modify the generic dataset endpoint to be specific to the current dataset.
+   *
+   * @param array $spec
+   *   The original spec.
+   * @param string $identifier
+   *   Dataset uuid.
+   *
+   * @return array
+   *   Spec with dataset-specific metastore get endpoint.
+   */
+  private function modifyDatasetEndpoint(array $spec, string $identifier) {
     if (isset($spec['paths']['/api/1/metastore/schemas/dataset/items/{identifier}'])) {
+      unset($spec['paths']['/api/1/metastore/schemas/dataset/items/{identifier}']['get']['parameters']);
+      // Replace the dataset uuid placeholder.
       $spec['paths']['/api/1/metastore/schemas/dataset/items/' . $identifier] = $spec['paths']['/api/1/metastore/schemas/dataset/items/{identifier}'];
       unset($spec['paths']['/api/1/metastore/schemas/dataset/items/{identifier}']);
+      // Keep only the tags needed, starting with the dataset tag.
+      $spec['tags'][] = ["name" => "Dataset"];
     }
+    return $spec;
+  }
 
-    // Replace the sql endpoint query placeholder.
-    $spec = $this->replaceDistributions($spec, $identifier);
-
-    return $this->getResponse($spec);
+  /**
+   * Modify the generic sql endpoint to be specific to the current dataset.
+   *
+   * @param array $spec
+   *   The original spec.
+   * @param string $identifier
+   *   Dataset uuid.
+   *
+   * @return array
+   *   Spec with dataset-specific datastore sql endpoint.
+   */
+  private function modifySqlEndpoint(array $spec, string $identifier) {
+    if (isset($spec['paths']['/api/1/datastore/sql'])) {
+      unset($spec['paths']['/api/1/datastore/sql']['get']['parameters']);
+      $spec = $this->replaceDistributions($spec, $identifier);
+      $spec['tags'][] = ["name" => "SQL Query"];
+    }
+    return $spec;
   }
 
   /**

modules/custom/dkan_metastore/tests/src/Unit/WebServiceApiDocsTest.php

@@ -22,21 +22,10 @@ class WebServiceApiDocsTest extends TestCase {
   public function testGetDatasetSpecific() {
     $mockChain = $this->getCommonMockChain();
 
-    // Test against ./docs/dkan_api_openapi_spec.yml.
-    $endpointsToKeep = [
-      // Target paths.
-      '/api/1/metastore/schemas/dataset/items/{identifier}' => ['get'],
-      '/api/1/datastore/sql' => ['get'],
-      // Non-existent operation.
-      '/api/1/some/other/path' => ['get'],
-      // Non-existent path.
-      'api/1/non/existent/path' => ['put'],
-    ];
-
     $controller = WebServiceApiDocs::create($mockChain->getMock());
-    $response = $controller->getDatasetSpecific(1, $endpointsToKeep);
+    $response = $controller->getDatasetSpecific(1);
 
-    $spec = '{"openapi":"3.0.1","info":{"title":"API Documentation","version":"Alpha"},"paths":{"\/api\/1\/metastore\/schemas\/dataset\/items\/1":{"get":{"summary":"Get this dataset","tags":["Dataset"],"responses":{"200":{"description":"Ok"}}}}},"tags":[{"name":"Dataset"},{"name":"SQL Query"}]}';
+    $spec = '{"openapi":"3.0.1","info":{"title":"API Documentation","version":"Alpha"},"tags":[{"name":"Dataset"},{"name":"SQL Query"}],"paths":{"\/api\/1\/datastore\/sql":{"get":{"summary":"Query resources","tags":["SQL Query"],"responses":{"200":{"description":"Ok"}}}},"\/api\/1\/metastore\/schemas\/dataset\/items\/1":{"get":{"summary":"Get this dataset","tags":["Dataset"],"responses":{"200":{"description":"Ok"}}}}}}';
 
     $this->assertEquals($spec, $response->getContent());
   }
@@ -46,6 +35,7 @@ public function testGetDatasetSpecific() {
    */
   private function getCommonMockChain() {
     $serializer = new Yaml();
+    // Test against ./docs/dkan_api_openapi_spec.yml.
     $yamlSpec = file_get_contents(__DIR__ . "/docs/dkan_api_openapi_spec.yml");
 
     $mockChain = new Chain($this);

modules/custom/dkan_metastore/tests/src/Unit/docs/dkan_api_openapi_spec.yml

@@ -2,6 +2,10 @@ openapi: 3.0.1
 info:
   title: API Documentation
   version: Alpha
+tags:
+  - name: Dataset
+  - name: SQL Query
+  - name: Another Tag
 paths:
   /api/1/metastore/schemas/dataset/items/{identifier}:
     get:
@@ -26,6 +30,16 @@ paths:
           description: Ok
     # Though an empty verb invalidates the spec, test its removal by dataset-specific docs.
     post:
+  /api/1/datastore/sql:
+    get:
+      summary: Query resources
+      tags:
+        - SQL Query
+      parameters:
+        - $ref: '#/components/parameters/query'
+      responses:
+        '200':
+          description: Ok
   /api/1/some/other/path:
     patch:
       summary: This path and operation should not be present in dataset-specific docs.