Adding more test cases and making more improvements

.gitignore

@@ -13,3 +13,9 @@ vendor/**/composer.lock
 /vue/dist/*.common.js
 /vue/dist/*.map
 /vue/dist/*.development.*
+/tmp/specs/*
+!/tmp/specs/.gitkeep
+/tmp/annotations/*
+!/tmp/annotations/.gitkeep
+/tmp/responses/*
+!/tmp/responses/.gitkeep

OpenApiDocs.php

@@ -11,6 +11,11 @@
 
 class OpenApiDocs extends \Piwik\Plugin
 {
+    public const DEFAULT_SPEC_VERSION = '1.0.0';
+    public const GENERATED_ANNOTATIONS_PATH = '/tmp/annotations/';
+    public const EXAMPLE_RESPONSES_PATH = '/tmp/responses/';
+    public const GENERATED_SPECS_PATH = '/tmp/specs/';
+
     public function registerEvents()
     {
         return [];

Specs/SpecGenerator.php

@@ -15,7 +15,7 @@
 use Piwik\Log\LoggerInterface;
 use Piwik\Log\NullLogger;
 use Piwik\Plugin\Manager;
-use Piwik\Plugins\OpenApiDocs\Annotations\AnnotationGenerator;
+use Piwik\Plugins\OpenApiDocs\OpenApiDocs;
 use Piwik\SettingsPiwik;
 use Piwik\Validators\BaseValidator;
 use Piwik\Validators\NotEmpty;
@@ -30,55 +30,83 @@ public function __construct()
         }
     }
 
-    public function generatePluginDoc(string $pluginName, string $format = 'json', string $version = '1.0.0', bool $writeToFile = false): string
+    /**
+     * Generate an OpenAPI spec for a single plugin.
+     *
+     * @param string $pluginName
+     * @param string $format
+     * @param string $version
+     * @param bool $writeToFile
+     *
+     * @return string
+     * @throws \Exception
+     */
+    public function generatePluginDoc(string $pluginName, string $format = 'json', string $version = OpenApiDocs::DEFAULT_SPEC_VERSION, bool $writeToFile = false): string
     {
         BaseValidator::check('plugin', $pluginName, [new NotEmpty()]);
-        Manager::getInstance()->checkIsPluginActivated($pluginName);
 
+        return $this->generateSpec(explode(',', $pluginName), $format, $version, $writeToFile);
+    }
+
+    /**
+     * Generate an OpenAPI spec for one or more plugins.
+     *
+     * @param array $pluginNames
+     * @param string $format
+     * @param string $version
+     * @param bool $writeToFile
+     *
+     * @return string
+     * @throws \Piwik\Exception\DI\DependencyException
+     * @throws \Piwik\Exception\DI\NotFoundException
+     * @throws \Piwik\Exception\PluginDeactivatedException
+     */
+    public function generateSpec(array $pluginNames, string $format = 'json', string $version = OpenApiDocs::DEFAULT_SPEC_VERSION, bool $writeToFile = false): string
+    {
+        BaseValidator::check('pluginNames', $pluginNames, [new NotEmpty()]);
         $currentPluginDir = Manager::getInstance()::getPluginDirectory('OpenApiDocs');
-        $pluginDir = Manager::getInstance()::getPluginDirectory($pluginName);
-        $pluginSpecDir = $pluginDir . '/OpenApi/Specs';
-        $pluginSpecPath = $pluginSpecDir . '/' . $pluginName . '_v' . $version . '.' . strtolower($format);
-        // If the directory doesn't exist yet, create it
-        if ($writeToFile && !is_dir($pluginSpecDir)) {
-            mkdir($pluginSpecDir, 0777, true);
-        }
 
-        // Check if the API class has been annotated and use the generated annotations file if it hasn't
-        $pluginAnnotationsSource = $pluginDir . '/API.php';
-        $openapi = (new Generator(StaticContainer::get(NullLogger::class)))->generate([
-            $pluginAnnotationsSource,
-        ]);
-        if (trim($openapi->toYaml()) === 'openapi: ' . OpenApi::DEFAULT_VERSION) {
-            $pluginAnnotationDir = $pluginDir . '/OpenApi/Annotations';
-            $pluginAnnotationPath = $pluginAnnotationDir . '/GeneratedAnnotations.php';
-            $pluginAnnotationsSource = $pluginAnnotationPath;
-            // If the generated file doesn't exist yet, generate one
-            if (!is_dir($pluginAnnotationDir) || !file_exists($pluginAnnotationPath)) {
-                (StaticContainer::get(AnnotationGenerator::class))->generatePluginApiAnnotations($pluginName, true);
+        $pluginDirs = [];
+        foreach ($pluginNames as $pluginName) {
+            BaseValidator::check('pluginName', $pluginName, [new NotEmpty()]);
+            Manager::getInstance()->checkIsPluginActivated($pluginName);
+
+            $pluginDir = Manager::getInstance()::getPluginDirectory($pluginName);
+            $pluginAnnotationsSource = $pluginDir . '/API.php';
+            $openapi = (new Generator(StaticContainer::get(NullLogger::class)))->generate([
+                $pluginAnnotationsSource,
+            ]);
+            if (trim($openapi->toYaml()) === 'openapi: ' . OpenApi::DEFAULT_VERSION) {
+                throw new \Exception("The $pluginName plugin's API class does not appear to be annotated yet.");
             }
+            $pluginDirs[$pluginName] = $pluginAnnotationsSource;
         }
 
         $generator = new Generator(StaticContainer::get(LoggerInterface::class));
-
-        $openapi = $generator->generate([
+        $openapi = $generator->generate(array_merge([
             $currentPluginDir . '/Annotations/GlobalApiComponents.php',
-            $pluginAnnotationsSource,
-        ]);
+        ], $pluginDirs));
 
-        // Update title with plugin name
-        $openapi->info->title .= ' for ' . $pluginName . ' plugin';
+        $specFileBaseName = 'matomo';
+        // If there's only one plugin, name the spec after the plugin
+        if (count($pluginNames) === 1) {
+            // Update title with plugin name
+            $openapi->info->title .= ' for ' . $pluginNames[0] . ' plugin';
+            $specFileBaseName = $pluginNames[0];
+        }
 
-        $openapi->info->version = $version ?: '1.0.0';
+        $openapi->info->version = $version ?: OpenApiDocs::DEFAULT_SPEC_VERSION;
 
         // Remove the current server so that it isn't used when saving the spec file. It should only leave demo
         if ($writeToFile && is_array($openapi->servers) && count($openapi->servers) > 1) {
             unset($openapi->servers[0]);
             $openapi->servers = array_values($openapi->servers);
         }
 
-        $specContents = strtolower($format) === 'yaml' ? $openapi->toYaml() : $openapi->toJson();
+        $lowercaseFormat = strtolower($format);
+        $specContents = $lowercaseFormat === 'yaml' ? $openapi->toYaml() : $openapi->toJson();
         if ($writeToFile) {
+            $pluginSpecPath = $currentPluginDir . OpenApiDocs::GENERATED_SPECS_PATH . $specFileBaseName . '_openapi_spec_v' . $version . '.' . $lowercaseFormat;
             file_put_contents($pluginSpecPath, $specContents);
         }
 

phpcs.xml

@@ -9,6 +9,7 @@
 
     <exclude-pattern>tests/javascript/*</exclude-pattern>
     <exclude-pattern>*/vendor/*</exclude-pattern>
+    <exclude-pattern>*/tmp/*</exclude-pattern>
 
     <rule ref="Matomo"></rule>
 

tests/Resources/ExampleResponsesNormalised/ExamplesSchemasByType.json

@@ -40,19 +40,19 @@
     "json": "{\"@OA\\\\Schema\":{\"0\":\"type=\\\"array\\\",\",\"@OA\\\\Items\":{\"0\":\"type=\\\"object\\\",\",\"1\":\"additionalProperties=true,\",\"@OA\\\\Property\":[\"type=\\\"object\\\",\",\"@OA\\\\Property(property=\\\"severity\\\", type=\\\"string\\\")\",\"@OA\\\\Property(property=\\\"tag\\\", type=\\\"string\\\")\",\"@OA\\\\Property(property=\\\"datetime\\\", type=\\\"string\\\")\",\"@OA\\\\Property(property=\\\"requestId\\\", type=\\\"string\\\")\",\"@OA\\\\Property(property=\\\"message\\\", type=\\\"string\\\")\"]}}}"
   },
   "LogViewer.getAvailableLogReaders": {
-    "xml": "{\"@OA\\\\Schema\":[\"type=\\\"object\\\",\",\"@OA\\\\Xml(name=\\\"result\\\"),\",{\"@OA\\\\Property\":{\"0\":\"property=\\\"row\\\",\",\"1\":\"type=\\\"array\\\",\",\"@OA\\\\Items\":[\"type=\\\"string\\\"\"]}}]}",
+    "xml": "{\"@OA\\\\Schema\":[\"type=\\\"object\\\",\",\"@OA\\\\Xml(name=\\\"result\\\"),\",{\"@OA\\\\Property\":{\"0\":\"property=\\\"row\\\",\",\"1\":\"type=\\\"array\\\",\",\"@OA\\\\Items\":[\"type=\\\"object\\\",\",\"@OA\\\\Xml(name=\\\"row\\\"),\",\"additionalProperties=true,\"]}}]}",
     "json": "{\"@OA\\\\Schema\":[\"type=\\\"array\\\",\",\"@OA\\\\Items()\"]}"
   },
   "LogViewer.getConfiguredLogReaders": {
     "xml": "{\"@OA\\\\Schema\":[\"type=\\\"object\\\",\",\"@OA\\\\Xml(name=\\\"result\\\"),\"]}",
     "json": "{\"@OA\\\\Schema\":[\"type=\\\"array\\\",\",\"@OA\\\\Items()\"]}"
   },
   "LogViewer.getLogConfig": {
-    "xml": "{\"@OA\\\\Schema\":[\"type=\\\"object\\\",\",\"@OA\\\\Xml(name=\\\"result\\\"),\",{\"@OA\\\\Property\":[\"property=\\\"log_writers\\\",\",\"type=\\\"object\\\",\",{\"@OA\\\\Property\":{\"0\":\"property=\\\"row\\\",\",\"1\":\"type=\\\"array\\\",\",\"@OA\\\\Items\":[\"type=\\\"string\\\"\"]}}]}]}",
+    "xml": "{\"@OA\\\\Schema\":[\"type=\\\"object\\\",\",\"@OA\\\\Xml(name=\\\"result\\\"),\",{\"@OA\\\\Property\":[\"property=\\\"log_writers\\\",\",\"type=\\\"object\\\",\",{\"@OA\\\\Property\":{\"0\":\"property=\\\"row\\\",\",\"1\":\"type=\\\"array\\\",\",\"@OA\\\\Items\":[\"type=\\\"object\\\",\",\"@OA\\\\Xml(name=\\\"row\\\"),\",\"additionalProperties=true,\"]}}]}]}",
     "json": "{\"@OA\\\\Schema\":{\"0\":\"type=\\\"object\\\",\",\"@OA\\\\Property\":[\"property=\\\"log_writers\\\",\",\"type=\\\"array\\\",\",\"@OA\\\\Items()\"],\"1\":\"@OA\\\\Property(property=\\\"log_level\\\", type=\\\"string\\\")\",\"2\":\"@OA\\\\Property(property=\\\"logger_file_path\\\", type=\\\"string\\\")\",\"3\":\"@OA\\\\Property(property=\\\"logger_syslog_ident\\\", type=\\\"string\\\")\"}}"
   },
   "CustomAlerts.getAlert": {
-    "xml": "{\"@OA\\\\Schema\":[\"type=\\\"object\\\",\",\"@OA\\\\Xml(name=\\\"result\\\"),\"]}",
+    "xml": "{\"@OA\\\\Schema\":[\"type=\\\"object\\\",\",\"@OA\\\\Xml(name=\\\"result\\\"),\",{\"@OA\\\\Property\":[\"property=\\\"report_mediums\\\",\",\"type=\\\"object\\\",\"]},{\"@OA\\\\Property\":[\"property=\\\"id_sites\\\",\",\"type=\\\"object\\\",\"]}]}",
     "json": "{\"@OA\\\\Schema\":[\"type=\\\"array\\\",\",\"@OA\\\\Items()\"]}"
   },
   "CustomAlerts.getAlerts": {
@@ -68,7 +68,7 @@
     "json": "{\"@OA\\\\Schema\":{\"0\":\"type=\\\"array\\\",\",\"@OA\\\\Items\":{\"0\":\"type=\\\"object\\\",\",\"1\":\"additionalProperties=true,\",\"@OA\\\\Property\":{\"0\":\"type=\\\"object\\\",\",\"1\":\"@OA\\\\Property(property=\\\"idtriggered\\\", type=\\\"integer\\\")\",\"2\":\"@OA\\\\Property(property=\\\"idalert\\\", type=\\\"integer\\\")\",\"3\":\"@OA\\\\Property(property=\\\"idsite\\\", type=\\\"integer\\\")\",\"4\":\"@OA\\\\Property(property=\\\"ts_triggered\\\", type=\\\"string\\\")\",\"5\":\"@OA\\\\Property(property=\\\"ts_last_sent\\\", type=\\\"string\\\")\",\"6\":\"@OA\\\\Property(property=\\\"value_old\\\", type=\\\"string\\\")\",\"7\":\"@OA\\\\Property(property=\\\"value_new\\\", type=\\\"string\\\")\",\"8\":\"@OA\\\\Property(property=\\\"name\\\", type=\\\"string\\\")\",\"9\":\"@OA\\\\Property(property=\\\"login\\\", type=\\\"string\\\")\",\"10\":\"@OA\\\\Property(property=\\\"period\\\", type=\\\"string\\\")\",\"11\":\"@OA\\\\Property(property=\\\"report\\\", type=\\\"string\\\")\",\"12\":\"@OA\\\\Property(property=\\\"report_condition\\\", type={\\\"string\\\", \\\"number\\\", \\\"integer\\\", \\\"boolean\\\", \\\"array\\\", \\\"object\\\", \\\"null\\\"})\",\"13\":\"@OA\\\\Property(property=\\\"report_matched\\\", type={\\\"string\\\", \\\"number\\\", \\\"integer\\\", \\\"boolean\\\", \\\"array\\\", \\\"object\\\", \\\"null\\\"})\",\"@OA\\\\Property\":[\"property=\\\"id_sites\\\",\",\"type=\\\"array\\\",\",\"@OA\\\\Items()\"],\"14\":\"@OA\\\\Property(property=\\\"metric\\\", type=\\\"string\\\")\",\"15\":\"@OA\\\\Property(property=\\\"metric_condition\\\", type=\\\"string\\\")\",\"16\":\"@OA\\\\Property(property=\\\"metric_matched\\\", type=\\\"integer\\\")\",\"17\":\"@OA\\\\Property(property=\\\"compared_to\\\", type=\\\"integer\\\")\",\"18\":\"@OA\\\\Property(property=\\\"email_me\\\", type=\\\"integer\\\")\",\"19\":\"@OA\\\\Property(property=\\\"slack_channel_id\\\", type={\\\"string\\\", \\\"number\\\", \\\"integer\\\", \\\"boolean\\\", \\\"array\\\", \\\"object\\\", \\\"null\\\"})\"}}}}"
   },
   "CustomDimensions.getCustomDimension": {
-    "xml": "{\"@OA\\\\Schema\":[\"type=\\\"object\\\",\",\"@OA\\\\Xml(name=\\\"result\\\"),\",{\"@OA\\\\Property\":{\"0\":\"property=\\\"row\\\",\",\"1\":\"type=\\\"array\\\",\",\"@OA\\\\Items\":[\"type=\\\"string\\\"\"]}}]}",
+    "xml": "{\"@OA\\\\Schema\":[\"type=\\\"object\\\",\",\"@OA\\\\Xml(name=\\\"result\\\"),\",{\"@OA\\\\Property\":{\"0\":\"property=\\\"row\\\",\",\"1\":\"type=\\\"array\\\",\",\"@OA\\\\Items\":[\"type=\\\"object\\\",\",\"@OA\\\\Xml(name=\\\"row\\\"),\",\"additionalProperties=true,\"]}}]}",
     "json": "{\"@OA\\\\Schema\":{\"0\":\"type=\\\"array\\\",\",\"@OA\\\\Items\":{\"0\":\"type=\\\"object\\\",\",\"1\":\"additionalProperties=true,\",\"@OA\\\\Property\":{\"0\":\"type=\\\"object\\\",\",\"1\":\"@OA\\\\Property(property=\\\"label\\\", type=\\\"string\\\")\",\"2\":\"@OA\\\\Property(property=\\\"nb_uniq_visitors\\\", type=\\\"string\\\")\",\"3\":\"@OA\\\\Property(property=\\\"nb_visits\\\", type=\\\"string\\\")\",\"4\":\"@OA\\\\Property(property=\\\"nb_actions\\\", type=\\\"string\\\")\",\"5\":\"@OA\\\\Property(property=\\\"max_actions\\\", type=\\\"integer\\\")\",\"6\":\"@OA\\\\Property(property=\\\"sum_visit_length\\\", type=\\\"string\\\")\",\"7\":\"@OA\\\\Property(property=\\\"bounce_count\\\", type=\\\"string\\\")\",\"8\":\"@OA\\\\Property(property=\\\"nb_visits_converted\\\", type=\\\"string\\\")\",\"@OA\\\\Property\":{\"0\":\"property=\\\"goals\\\",\",\"1\":\"type=\\\"object\\\",\",\"@OA\\\\Property\":[\"property=\\\"idgoal=8\\\",\",\"type=\\\"object\\\",\",\"@OA\\\\Property(property=\\\"nb_conversions\\\", type=\\\"integer\\\")\",\"@OA\\\\Property(property=\\\"nb_visits_converted\\\", type=\\\"integer\\\")\",\"@OA\\\\Property(property=\\\"revenue\\\", type=\\\"integer\\\")\"]},\"9\":\"@OA\\\\Property(property=\\\"nb_conversions\\\", type=\\\"integer\\\")\",\"10\":\"@OA\\\\Property(property=\\\"revenue\\\", type=\\\"integer\\\")\",\"11\":\"@OA\\\\Property(property=\\\"avg_time_on_site\\\", type=\\\"integer\\\")\",\"12\":\"@OA\\\\Property(property=\\\"bounce_rate\\\", type=\\\"string\\\")\",\"13\":\"@OA\\\\Property(property=\\\"nb_actions_per_visit\\\", type={\\\"string\\\", \\\"number\\\", \\\"integer\\\", \\\"boolean\\\", \\\"array\\\", \\\"object\\\", \\\"null\\\"})\",\"14\":\"@OA\\\\Property(property=\\\"segment\\\", type=\\\"string\\\")\"}}}}"
   },
   "CustomDimensions.getConfiguredCustomDimensions": {

tests/Unit/AnnotationGeneratorTest.php

@@ -442,6 +442,56 @@ public function getTestDataForBuildParameterAnnotationData(): iterable
             'default' => 'Piwik\API\NoDefaultValue',
             'example' => '',
         ]];
+        yield 'should show one type when docInfo has two types and one is bool' => ['someParam', [], [
+            'type' => 'string|bool',
+        ], [
+            'name' => 'someParam',
+            'types' => ['string' => null],
+            'description' => '',
+            'required' => 'true',
+            'default' => 'Piwik\API\NoDefaultValue',
+            'example' => '',
+        ]];
+        yield 'should remove bool type when docInfo has more than 2 types and one is bool' => ['someParam', [], [
+            'type' => 'string|int|bool',
+        ], [
+            'name' => 'someParam',
+            'types' => ['string' => null, 'integer' => null],
+            'description' => '',
+            'required' => 'true',
+            'default' => 'Piwik\API\NoDefaultValue',
+            'example' => '',
+        ]];
+        yield 'should remove bool type regardless of spacing around pipe' => ['someParam', [], [
+            'type' => 'string | int | bool',
+        ], [
+            'name' => 'someParam',
+            'types' => ['string' => null, 'integer' => null],
+            'description' => '',
+            'required' => 'true',
+            'default' => 'Piwik\API\NoDefaultValue',
+            'example' => '',
+        ]];
+        yield 'should remove bool type regardless of spacing and order' => ['someParam', [], [
+            'type' => 'bool | string',
+        ], [
+            'name' => 'someParam',
+            'types' => ['string' => null],
+            'description' => '',
+            'required' => 'true',
+            'default' => 'Piwik\API\NoDefaultValue',
+            'example' => '',
+        ]];
+        yield 'should remove bool type even when type hints are wrapped by parenthesis' => ['someParam', [], [
+            'type' => '(bool | string)',
+        ], [
+            'name' => 'someParam',
+            'types' => ['string' => null],
+            'description' => '',
+            'required' => 'true',
+            'default' => 'Piwik\API\NoDefaultValue',
+            'example' => '',
+        ]];
         yield 'should allow multiple types when metadata type is string' => ['someParam', [
             'type' => 'string',
         ], [
@@ -454,6 +504,31 @@ public function getTestDataForBuildParameterAnnotationData(): iterable
             'default' => 'Piwik\API\NoDefaultValue',
             'example' => '',
         ]];
+        yield 'should allow multiple types when metadata type is bool and doc type is piped' => ['someParam', [
+            'type' => 'bool',
+        ], [
+            'type' => 'string|int|bool',
+        ], [
+            'name' => 'someParam',
+            'types' => ['string' => null, 'integer' => null],
+            'description' => '',
+            'required' => 'true',
+            'default' => 'Piwik\API\NoDefaultValue',
+            'example' => '',
+        ]];
+        yield 'should allow multiple types when metadata type is bool and doc type is piped even if default is bool' => ['someParam', [
+            'type' => 'bool',
+            'default' => false,
+        ], [
+            'type' => 'string|int|bool',
+        ], [
+            'name' => 'someParam',
+            'types' => ['string' => null, 'integer' => null],
+            'description' => '',
+            'required' => 'false',
+            'default' => 'false',
+            'example' => '',
+        ]];
         yield 'should not allow multiple types when metadata type is specified' => ['someParam', [
             'type' => 'integer',
         ], [
@@ -661,8 +736,16 @@ public function testBuildPropertyAnnotationFromJsonExample(): void
 
     public function testBuildSchemaAnnotationFromXmlExample(): void
     {
-        // TODO - buildSchemaAnnotationFromXmlExample method
-        $this->expectNotToPerformAssertions();
+        $normalisedMap = $this->getExampleResponsesMap();
+        $schemasMap = $this->getExampleResponsesMap(true);
+        foreach (self::EXAMPLE_API_ENDPOINTS as $endpoint) {
+            $normalisedString = $normalisedMap[$endpoint]['xml'] ?? '';
+            $this->assertNotEmpty($normalisedString, 'The normalised example response should not be empty for endpoint: ' . $endpoint);
+            $normalisedObject = json_decode($normalisedString, true) ?? [];
+            $this->assertNotEmpty($normalisedObject, 'The decoded example response should not be empty for endpoint: ' . $endpoint);
+            $expected = json_decode($schemasMap[$endpoint]['xml'] ?? '', true) ?? [];
+            $this->assertEquals($expected, $this->annotationGenerator->buildSchemaAnnotationFromXmlExample($normalisedObject), "The XML schema was not as expected for endpoint $endpoint.");
+        }
     }
 
     public function testBuildPropertyAnnotationFromXmlExample(): void