Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removing schema and routes check when request comes from web and hotReload is set to false to avoid unnecessary use of resources. #544

Conversation

manuelpeiso
Copy link
Contributor

The SwaggerFactory creates an instance of RouteScanner and also an instance of ModelScanner. No matter what, the RouteScanner is going to read all the routes and the ModelScanner is going to iterate over all the tables.
This behavior is quite rare when the hotReload is set to false, because the expected behavior would be to read the swagger.json and return the proper view (why to check routes and schema if we are not generating the swagger.json?)

This PR goal is to avoid this, avoid to load routes and schema in the swagger instance when the request is coming from the web and the hotReload configuration key is set to false.
The CLI requests are excluded because even when hotReload is set to false we need to check routes and schema when, for example, the command bin/cake swagger bake is execute.

src/Lib/Route/RouteScanner.php Outdated Show resolved Hide resolved
src/Lib/Route/RouteScanner.php Outdated Show resolved Hide resolved
@cnizzardini
Copy link
Owner

As noted in #543 I will not accept this PR as-is. I feel the potential for BC breaks is too high. If you are keen on introducing a programmatic solution to this issue, then my preference would likely be a configuration option that explicitly controls the behavior you are seeking rather than relying on SAPI. This solution could probably be added to your existing PR with minimal changes and with the added benefit of no potential for BC breaks.

@cnizzardini
Copy link
Owner

cnizzardini commented Feb 27, 2024

Also, I believe the issue you have identified here was previously identified and has been resolved in version 3.x of this library. I elected to not port it down because of some reason, probably laziness. Ultimately, I moved the poorly performing operations out of the constructor (bad coding practice anyways) and handled building openapi this way:

https://github.com/cnizzardini/cakephp-swagger-bake/blob/master/src/Lib/Service/OpenApiControllerService.php

I believe this is basically what you've accomplished in this PR in a roundabout way.

@cnizzardini cnizzardini closed this Mar 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants