Improving annotation generation by snake14 · Pull Request #6 · matomo-org/plugin-ApiReference

snake14 · 2025-08-29T07:37:59Z

Description

Making improvements to the annotation generation command. I realised that I should probably get my changes so far reviewed, so I'm creating this PR and branching from it for additional changes.

Issue No

PG-4396

Steps to Replicate the Issue

Run the command like ddev matomo:console openapidocs:generate-annotations --plugin=CustomAlerts to see the annotations for all of the API methods of that plugin generated.
It may take longer since it's making curl calls to demo.matomo.cloud to try and include the examples.
Since CustomAlerts endpoints require the user to be logged in, the lookups will fail and the URL will be used as the example. The field containing the URL is supported by the OpenAPI specs, but Swagger UI doesn't support it yet.

Checklist

[✔] Tested locally or on demo2/demo3?
[✖] New test case added/updated?
[NA] Are all newly added texts included via translation?
[NA] Are text sanitized properly? (Eg use of v-text v/s v-html for vue)
[✖] Version bumped?

james-hill-matomo · 2025-08-31T21:57:25Z

@snake14 When testing CustomReports, I get an error:

ddev matomo:console openapidocs:generate-annotations --plugin=CustomReports

            mediaType="application/vnd.ms-excel",
            @OA\Examples(
                example="tsvDemoLink",
                summary="Example tsv",
                value="Error: Data structure returned is not convertible in the requested format: Multidimensional column values not supported. Found unexpected array value for column 'subcategories' in row '0': 'array (
  0 =&gt; 
  array (
    'uniqueId' =&gt; 'customdimension2',
    'name' =&gt; 'Page Author',
  ),
  1 =&gt; 
  array (
    'uniqueId' =&gt; 'customdimension4',
    'name' =&gt; 'Page Location',
  ),
  2 =&gt; 
  array (
    'uniqueId' =&gt; 'customdimension5',
    'name' =&gt; 'Page Type',
  ),
  3 =&gt; 
  array (
    'uniqueId' =&gt; 'VisitorInterest_Engagement',
    'name' =&gt; 'Engagement',
  ),
  4 =&gt; 
  array (
    'uniqueId' =&gt; 'Transitions_Transitions',
    'name' =&gt; 'Transitions',
  ),
  5 =&gt; 
  array (
    'uni'. Try to call this method with the parameters '&amp;format=original&amp;serialize=1'; you will get the original php data structure serialized."
            )
        )
    ),
    @OA\Response(response=400, ref="#/components/responses/BadRequest"),
    @OA\Response(response=401, ref="#/components/responses/Unauthorized"),
    @OA\Response(response=403, ref="#/components/responses/Forbidden"),
    @OA\Response(response=404, ref="#/components/responses/NotFound"),
    @OA\Response(response=500, ref="#/components/responses/ServerError"),
    @OA\Response(response="default", ref="#/components/responses/DefaultError"),
    x={"runtime"={"entry":"index.php","query":{"module":"API","method":"CustomReports.getAvailableCategories"}}}
)

james-hill-matomo · 2025-08-31T22:15:26Z

@snake14

$categoryId is (incorrectly) typed as string in the docblock, and in the function signature it defaults to false. Not sure the best way of solving this, or if we even should in the automated generation.

james-hill-matomo · 2025-08-31T22:20:23Z

@snake14 Default query param of '' is encoded as """"

@snake14 apologies - this CR isn't scoped to your changes well.

james-hill-matomo · 2025-08-31T22:23:44Z

+        $pluginConfigPath = $pluginDir . '/OpenApi/Annotations/config.php';
+        if (is_file($pluginConfigPath)) {
+            $pluginRules = require $pluginDir . '/OpenApi/Annotations/config.php';
+        }


Maybe nitpick (does the security scanner pick up on this?) but if this is always going to be arrays, it'd be much more secure to have this as json or yaml or something rather than requiring a php.

Are you planning for us to customise each plugin in pluginDir/OpenApi/Annotations/config.php instead of directly editing the file? Or is this for something else?

@james-hill-matomo As mentioned in Slack, I'm trying to remove the use of a config file. The main thing that I haven't determined how to do without a config is whether to use GET or POST for each API method. I guess I can just have everything show as GET for now.

james-hill-matomo · 2025-08-31T22:35:24Z


+        $mediaTypes = [];
+        // This simply reuses the example URLs used by the current documentation, but some endpoints don't work because authentication is required
+        // TODO - Come up with a way to demo examples for endpoints which require authentication. E.g. hit a live endpoint server-side and replace any potentially sensitive data...


That can stay out of scope for now. I don't think the demos will be so important if we can provide a live swagger that users can query their own data with easily.

james-hill-matomo · 2025-08-31T22:58:15Z

+        if ($default !== '') {
+            // TODO - Add some logic to only add default if it matches the type. E.g. false isn't a good default for string
+            $schemaMap[] = 'default="' . $default . '"';
+        }


Hah - Shame I didn't read this before my other comments about the defaults.

snake14 · 2025-09-01T01:32:52Z

@snake14 When testing CustomReports, I get an error:

ddev matomo:console openapidocs:generate-annotations --plugin=CustomReports

Ah. Yeah. Probably related to Matomo APIs returning 200 type responses when they should be 4xx or 5xx. I can add an extra check.

Edit: With the latest commit, annotations are built successfully for the CustomReports plugin.

AltamashShaikh · 2025-09-01T02:04:24Z

+
+    protected function getExampleIfAvailable(string $url): array
+    {
+        $ch = curl_init($url);


Can't we use Http::sendHttpRequestBy instead of using curl directly ?

@AltamashShaikh Possibly. I forgot that was an option and didn't see it when I was looking for examples in the codebase.

AltamashShaikh

@snake14 Left few comments overall looks good to me.

snake14 · 2025-09-01T05:20:54Z

@snake14 Default query param of '' is encoded as """"

I just tested Swagger UI and it shows Default: '' instead of Default: , which seems preferable to me.

snake14 · 2025-09-01T23:45:38Z

@james-hill-matomo @AltamashShaikh Any concerns with the last 2-3 commits or am I good to merge once #7 is merged in?

…lows

AltamashShaikh

@snake14 I can see the file being created with not-dry-run option

Pg 4396 add workflows

snake14 added 2 commits August 29, 2025 19:31

Improving annotation generation

6369e6a

Merge branch '5.x-dev' into PG-4396-generate-oa-annotations

76df266

snake14 added the Needs Review For pull requests that need a code review. label Aug 29, 2025

snake14 requested a review from a team August 29, 2025 07:40

snake14 added 3 commits September 1, 2025 09:15

Added PHPCS

47d423d

Fix code style issues

def9376

Adding PHPStan

75b5b9e

Add test YML

f2ba1a9

james-hill-matomo approved these changes Aug 31, 2025

View reviewed changes

snake14 added 6 commits September 1, 2025 11:10

Add unit test case

083a523

Exclude vendor directory from PHPStan

a28af33

Add more PHPStan files

c298361

Making a couple more PHPStan changes

cf4db37

Fixed pre-push issue

c117f4d

Temporarily have PHPStan ignore class

29b4efd

AltamashShaikh reviewed Sep 1, 2025

View reviewed changes

Comment thread Annotations/AnnotationGenerator.php

AltamashShaikh reviewed Sep 1, 2025

View reviewed changes

PR feedback and other fixes

ce0b03a

snake14 added 2 commits September 2, 2025 11:31

Add annotation file and use it for spec generation

ba528b2

A little more cleanup

9190943

snake14 and others added 3 commits September 2, 2025 12:22

Minor corrections

08fbf2b

Merge branch 'PG-4396-generate-oa-annotations' into PG-4396-add-workf…

0052902

…lows

Fixing code style issue after merging parent

52bc68e

snake14 mentioned this pull request Sep 2, 2025

Removing config file from previous iteration matomo-org/plugin-CustomAlerts#206

Merged

AltamashShaikh approved these changes Sep 2, 2025

View reviewed changes

Merge pull request #7 from matomo-org/PG-4396-add-workflows

1514976

Pg 4396 add workflows

snake14 merged commit 2bfe31a into 5.x-dev Sep 2, 2025
7 checks passed

snake14 deleted the PG-4396-generate-oa-annotations branch September 2, 2025 20:54

Conversation

snake14 commented Aug 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issue No

Steps to Replicate the Issue

Checklist

Uh oh!

james-hill-matomo commented Aug 31, 2025

Uh oh!

james-hill-matomo commented Aug 31, 2025

Uh oh!

james-hill-matomo commented Aug 31, 2025

Uh oh!

james-hill-matomo Aug 31, 2025

Choose a reason for hiding this comment

Uh oh!

james-hill-matomo Aug 31, 2025

Choose a reason for hiding this comment

Uh oh!

snake14 Sep 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

james-hill-matomo Aug 31, 2025

Choose a reason for hiding this comment

Uh oh!

james-hill-matomo Aug 31, 2025

Choose a reason for hiding this comment

Uh oh!

snake14 commented Sep 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

AltamashShaikh Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

snake14 Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

AltamashShaikh left a comment

Choose a reason for hiding this comment

Uh oh!

snake14 commented Sep 1, 2025

Uh oh!

snake14 commented Sep 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AltamashShaikh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

snake14 commented Aug 29, 2025 •

edited

Loading

snake14 Sep 1, 2025 •

edited

Loading

snake14 commented Sep 1, 2025 •

edited

Loading

snake14 commented Sep 1, 2025 •

edited

Loading