fix: use schemas only from request body

[vitormattos]

Scope

Have schemas that only exists at requestBody and at this case, if we don't fetch from body will throw an error "Can't resolve $ref" when run openapi-typescript build.

How to reproduce

• Create a new endpoint with an object as param

   /**
    * Route with param
    *
    * @param FakeParam $fake Fake param
    * @return DataResponse<Http::STATUS_OK, array<empty>, array{}>
    *
    * 200: Personal settings updated
    */
   #[OpenAPI]
   public function fakeMethod DataResponse {
   	return new DataResponse();
   }

• Add the FakeParam to ResponseDefinitions

    * @psalm-type NotificationsRequestProperty = array{
    *     publicKey: string,
    *     signature: string,
    * }
  

• Build with the follow command:

   npx openapi-typescript -t
  

Expected behavior

> [email protected] typescript:generate
> npx openapi-typescript -t

🚀 openapi@v1 → ./src/types/openapi/openapi.ts [141.7ms]

Actual behavior

> [email protected] typescript:generate
> npx openapi-typescript -t

 ✘  Can't resolve $ref
file:///var/www/html/apps-extra/dummyapp/node_modules/openapi-typescript/dist/lib/redoc.js:121
                throw new Error(problem.message);
                      ^

Error: Can't resolve $ref
    at validateAndBundle (file:///var/www/html/apps-extra/dummyapp/node_modules/openapi-typescript/dist/lib/redoc.js:121:23)
    at async openapiTS (file:///var/www/html/apps-extra/dummyapp/node_modules/openapi-typescript/dist/index.js:40:20)
    at async generateSchema (file:///var/www/html/apps-extra/dummyapp/node_modules/openapi-typescript/bin/cli.js:119:5)
    at async file:///var/www/html/apps-extra/dummyapp/node_modules/openapi-typescript/bin/cli.js:195:24
    at async Promise.all (index 0)
    at async main (file:///var/www/html/apps-extra/dummyapp/node_modules/openapi-typescript/bin/cli.js:181:5)

Node.js v20.15.1
Please manually regenerate the typescript OpenAPI models

Artifacts

Without fix

---

With fix

Tests

Considering that the test only build the json files and not the ts files, I didn't implemented tests.

[provokateurin]

Thanks for catching this.
A test is absolutely necessary to prevent future changes from accidentally reverting the fix.
Preferably the logic is not duplicated with the code above, since it is exactly the same and only operates on a different part of the spec.

[vitormattos]

Can I go ahead and implement a test to check if openapi-typescript will build without problems?

Preferably the logic is not duplicated with the code above, since it is exactly the same and only operates on a different part of the spec.

Could you comment more about this?

[provokateurin]

Can I go ahead and implement a test to check if openapi-typescript will build without problems?

Yes that would be nice, I thought about extending our current checks against existing repos by running openapi-typescript inside them if they use it. That should at least prevent regressions.

Could you comment more about this?

The logic is the same as a few lines above but complex enough that it should not be implemented twice.

[vitormattos]

Yes that would be nice, I thought about extending our current checks against existing repos by running openapi-typescript inside them if they use it. That should at least prevent regressions.

What you think about put this in a separated PR to don't mix with the context of this PR? I understand that tests are very important but would be best separate the scope to merge this fix more quickly.

[vitormattos]

The logic is the same as a few lines above but complex enough that it should not be implemented twice.

Did you have any suggestion to this? This file isn't object oriented, is a point that make the reuse of code a bit complex. To prevent a duplication of code, maybe the both loop could be moved to Helper class.

[provokateurin]

What you think about put this in a separated PR to don't mix with the context of this PR? I understand that tests are very important but would be best separate the scope to merge this fix more quickly.

Yes, but a simple test for this fix still needs to be added to this fix.

Did you have any suggestion to this? This file isn't object oriented, is a point that make the reuse of code a bit complex. To prevent a duplication of code, maybe the both loop could be moved to Helper class.

Can be done easily by putting all elements into a single array and then looping over them instead of looping over them separately.

[provokateurin]

I had a look at the implementation of Helpers::collectUsedRefs and since it just walks any array structure recursively it can be used directly instead of iterating over the media types first.
This makes everything a lot simpler.

[github-actions]

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)