-
Notifications
You must be signed in to change notification settings - Fork 6k
Feature/unify response handling #7577
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
Feature/unify response handling #7577
Conversation
This adds a more unified handling for response code to type mapping. This fixes the deserialization for non default responses, which in previously implementation were only based on exception logic. This approach assumes that when the specification defines an possible response code outside of the default one, the client should handle the result in the same way as the default, but still propagate the result as exception. This exception is a MappedException, as it's defined by specification. Any other unexpected response is still an ApiException. It's a BC break. - The model if ApiException is changed, as it's now more generic. - The client makes use of php 7.1 features, I assume it's a required step. (https://secure.php.net/supported-versions.php) This includes return type hinting. (For operations with 'void' only.)
This changes phpUnit to 6.0 / 7.0, according to php version requirement. I moved the code style dependencies to `suggest` as cs-fixer and phpcs are not a development requirement, they can be used to modify the code but only if wanted.
Asynchronous operations will now use the same mapping routines as the sync operations do. This makes sure that non default responses can be mapped too.
eeeebd6 to
9b06da0
Compare
The client is using php 7.1 syntax, I know this is another breaking change, but php 5 is almost dead.
Would it be possible to keep the PHP client working with PHP 5.x? This is not to say we encourage PHP developers not to upgrade to 7.x but seems like a lot of users are still using PHP 5.x
For Java, there's an option supportJava6 to make the Java API client compatible with JDK6 so that developers still have the option to run it in their environment if they can't JDK to the latest version for whatever reason.
Thank you for your suggestion!
I think that we should keep the PHP client works with currently supported versions.
PR checklist
./bin/to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.shand./bin/security/{LANG}-petstore.shif 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\.3.0.0branch for changes related to OpenAPI spec 3.0. Default:master.Description of the PR
This changes should uniform the response data deserialization process. I recently faced the problem that the expected response model for non default response codes like 404 where not exposed inside the ApiException. This was caused by the different handling of the initial data.
But as you can see there are some other changes, maybe we can see this as a suggestion - that we could discus.
asyncfunctions use the same response handling as thesyncimplementations now. This makes sure that we don't face different response data. The previous implementation missed any exception model mapping.php 7.1syntax, I know this is another breaking change, but php 5 is almost dead.ApiExceptionas base andMappedApiExceptionfor responses that are defined by specification.ApiException.I see some more possible things that I would like to discus with you.
ObjectSerializer?I'm sorry for the size of the PR and the BC breaks, but please see this as an suggestion.