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

New PHP Slim4 Server Generator #3658

Merged
merged 16 commits into from
Oct 21, 2019
Merged

New PHP Slim4 Server Generator #3658

merged 16 commits into from
Oct 21, 2019

Conversation

ybelenko
Copy link
Contributor

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\. If contributing template-only or documentation-only changes which will change sample output, be sure to build the project first.
  • Filed the PR against the correct branch: master, 4.1.x, 5.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

There are breaking changes in Slim 4.0.0 version. In issue #2851 we agreed to add new generator.

I had to update my own Slim Token Authentication fork because original repo author didn't accept my PRs and seems missing.

Upgrade Guide - Slim Framework

Already contains changes from #3621
Closes #2851

cc @jebentier, @dkarlovi, @mandrean, @jfastnacht, @ackintosh, @renepardon

Questions to maintainers

# START SCRIPT: bin/php-slim4-server-petstore.sh
[main] INFO  o.o.codegen.DefaultGenerator - OpenAPI Generator: php-slim4 (server)
[main] INFO  o.o.codegen.DefaultGenerator - Generator 'php-slim4' is considered stable.
  1. Generator 'php-slim4' is considered stable seems interesting, where can I change it? This option/flag is not documented as far as I know.
  2. What happened with security samples? I remember that I had to run bin/security/php-slim-server-petstore.sh before.
  3. Why Openapi 3.0 samples mapped to the same folder where 2.0 samples already exists. I mean ./bin/openapi3/php-slim-server-petstore.sh and ./bin/php-slim-server-petstore.sh scripts targets the same ./samples/server/petstore/php-slim directory.

@ybelenko ybelenko force-pushed the slim4 branch 2 times, most recently from 37f0636 to 9928112 Compare August 18, 2019 20:21
@ybelenko ybelenko marked this pull request as ready for review August 18, 2019 20:22
@ybelenko ybelenko force-pushed the slim4 branch 2 times, most recently from df56683 to c8c979c Compare August 23, 2019 21:56
@ybelenko
Copy link
Contributor Author

ybelenko commented Aug 24, 2019

I've just finished testing of all endpoints in petstore-with-fake-endpoints-models-for-testing.yaml file. There are few non-critical issues that I discovered.

  1. Slim framework doesn't parse request body in GET and DELETE methods which seems fair. However you still can get raw body data(param1=foo&param2=bar when Content-Type: application/x-www-form-urlencoded).
  2. Apache and Nginx webservers avoid underscores in HTTP headers, so api_key header received as api-key. Refs: Cannot retrieve all request headers using PHP slim framework and Why underscores are forbidden in HTTP header names. Maybe we should throw a warning in default generator when HTTP headers contains underscore?
  3. Current template can't distinguish file parameter in file type with Content-Type: multipart/form-data and binary format with Content-Type: application/x-www-form-urlencoded. I'll try to fix it in future patches. Solved in [core] Set isMultipart=true for multipart operations, fix possible NPE #3750

Conclusion is that generator is ready to be reviewed and merged.

@wing328
Copy link
Member

wing328 commented Aug 24, 2019

Maybe we should throw a warning in default generator when HTTP headers contains underscore?

I think there are configurable settings to make Apache/Nginx work properly with HTTP headers with underscore in the key name.

I don't mind showing a warning/info message reminding users about this default limitations - maybe showing such message starting in the PHP Slim generator that you've been working on.

@ybelenko ybelenko force-pushed the slim4 branch 3 times, most recently from 65d3e8d to c0e214c Compare August 26, 2019 18:50
@ybelenko ybelenko mentioned this pull request Aug 26, 2019
4 tasks
@ybelenko ybelenko moved this from In progress to Needs review in PHP Slim4 Server Generator Aug 26, 2019
Slim supports PSR-7 interfaces for its Request and Response objects.
Slim provides its own PSR-7 implementation so that it works out of the
box.
However, you are free to replace Slim’s default PSR-7 objects with
a third-party implementation.
[Ref] https://www.slimframework.com/docs/v4/concepts/value-objects.html
It somehow ended up with composerPackages and composerDevPackages
codegen variables and two additional functions. Hope, it's not too much.
Slim’s App settings used to be a part of the container and  they have
now been decoupled from it.

[Upgrade Guide](https://www.slimframework.com/docs/v4/start/upgrade.html)
Slim uses an optional dependency container to prepare, manage,
and inject application dependencies.
Slim supports containers that implement PSR-11
like [PHP-DI](http://php-di.org/doc/frameworks/slim.html).

[Upgrade Guide](https://www.slimframework.com/docs/v4/start/upgrade.html)
You can't write to response instance directly anymore,
need to retrieve body object first.

[Doc](https://www.slimframework.com/docs/v4/objects/response.html#the-response-body)
User can provide array or Container as constructor argument from now.
Small refactoring required to retrieve authentication options from
that argument.
This approach seems more flexible to me.
User can customize templates in favor of chosen PSR7 implementation.
It's easier to change Composer packages and their versions.
Since Slim 4.0.0 ServerRequest implementation doesn't have
getQueryParam and getParsedBodyParam methods anymore.
isMultipart codegen property is always false so far.
Hope that bug will be fixed soon.
@wing328
Copy link
Member

wing328 commented Nov 1, 2019

The php-slim4 generator has been included in the 4.2.0 release:

To generate PHP Slim4 server stub given an OpenAPI/Swagger specification file, please follow 3 simple steps below:

  1. Download the Java JAR (http://central.maven.org/maven2/org/openapitools/openapi-generator-cli/4.2.0/openapi-generator-cli-4.2.0.jar)
  2. Rename the JAR as "openapi-generator-cli.jar"
  3. Run the following command to generate a PHP Slim4 server stub for the Petstore API (https://raw.githubusercontent.com/OpenAPITools/openapi-generator/master/modules/openapi-generator/src/test/resources/2_0/petstore.yaml):

Mac/Linux:

Windows:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

[REQ] Update PHP Slim generator to 4.0.0-alpha
2 participants